Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/10510 )
Change subject: IMPALA-5552: Add support for authorized proxy groups ...................................................................... Patch Set 13: (3 comments) LGTM once my comments are addressed. http://gerrit.cloudera.org:8080/#/c/10510/13/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/10510/13/be/src/service/impala-server.cc@1394 PS13, Line 1394: if (groups.find("*") != groups.end()) { : return Status::OK(); : } nit: single line, no braces (same below) http://gerrit.cloudera.org:8080/#/c/10510/13/be/src/service/impala-server.cc@1406 PS13, Line 1406: end nit: PrettyPrinter::Print(MonotinicMillis() - start, TUnit::TIME_MS) remove L1404 http://gerrit.cloudera.org:8080/#/c/10510/11/tests/authorization/test_authorization.py File tests/authorization/test_authorization.py: http://gerrit.cloudera.org:8080/#/c/10510/11/tests/authorization/test_authorization.py@224 PS11, Line 224: er is 'hue' > Yes, for this particular test, the code path to getHadoopGroups will not be I see. That coverage could be achieved by modifying the args for the test test_group_impersonation (above) by adding --authorized_proxy_user_config=hue=something_that_is_never_used Can we do that instead? -- To view, visit http://gerrit.cloudera.org:8080/10510 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6953f89c293b06b72f523b11802232133d9d6cbb Gerrit-Change-Number: 10510 Gerrit-PatchSet: 13 Gerrit-Owner: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Greg Rahn <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Thu, 07 Jun 2018 00:05:57 +0000 Gerrit-HasComments: Yes
