Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8490 )
Change subject: IMPALA-2248: Make idle_session_timeout a query option ...................................................................... Patch Set 14: (1 comment) http://gerrit.cloudera.org:8080/#/c/8490/14/be/src/service/impala-server.h File be/src/service/impala-server.h: http://gerrit.cloudera.org:8080/#/c/8490/14/be/src/service/impala-server.h@452 PS14, Line 452: friend class SessionState; > Well, I feel here that making SessionState a friend class so that the Regis I actually made the register/unregister functions private in response to Michael's comment. I think it's reasonable that timeouts can only be registered via SessionState::SetTimeout(). I agree that it is unfortunate that SessionState now accesses all members of ImpalaServer. However, friend declarations only extend the access of private fields, but do not break the access boundary. Making a member public allows everybody to depend on that member. But, it might be not a huge issue in application code. SessionState is defined inside the class definition of ImpalaServer, therefore making it a friend is not a huge problem either in my opinion. It is aligned with the google style guide as well: https://google.github.io/styleguide/cppguide.html#Friends -- To view, visit http://gerrit.cloudera.org:8080/8490 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e Gerrit-Change-Number: 8490 Gerrit-PatchSet: 14 Gerrit-Owner: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Reviewer: Attila Jeges <atti...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Gabor Kaszab <gaborkas...@cloudera.com> Gerrit-Reviewer: Laszlo Gaal <laszlo.g...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Thu, 14 Dec 2017 14:06:41 +0000 Gerrit-HasComments: Yes
