Dan Hecht has posted comments on this change. Change subject: IMPALA-4965: Authorize access to runtime profile and exec summary ......................................................................
Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/7064/7/be/src/service/client-request-state.h File be/src/service/client-request-state.h: Line 218: const ClientRequestState& request_state); > We need two implementations of CheckProfileAccess(), one that works against The real logic is already consolidated in the common CheckProfileAccess() function that takes both users and bool. These friend functions just pull out So how about we just get rid of these two friend functions and call directly? They don't seem to really add much (there are only two callsites each and from the same file) and it seems clearer anyway to see where the two user values come. If you really want to keep the wrappers, why do they need to be 'friend'? Isn't the state they access public? The real problem here is that we have two classes to represent the same info. Down the road we should get rid of QueryStateRecord. -- To view, visit http://gerrit.cloudera.org:8080/7064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2255d587367c2d328590ae8534a5406c4b0c9b15 Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Marcel Kornacker <mar...@cloudera.com> Gerrit-HasComments: Yes
