Austin Nobis has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12901 )

Change subject: IMPALA-8309: add user authorization_provider flag
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12901/2/be/src/service/frontend.cc
File be/src/service/frontend.cc:

http://gerrit.cloudera.org:8080/#/c/12901/2/be/src/service/frontend.cc@41
PS2, Line 41:     "sentry",
> Honest question: why?
It's a nit so it is not something that needs to be fixed for the change to be 
merged. I just noticed all the other DEFINE_string in this file had the default 
value on the same line so I pointed it out.


http://gerrit.cloudera.org:8080/#/c/12901/4/tests/authorization/test_ranger.py
File tests/authorization/test_ranger.py:

http://gerrit.cloudera.org:8080/#/c/12901/4/tests/authorization/test_ranger.py@38
PS4, Line 38:                   "--authorization_provider=ranger")
> I changed these to use the new flag as I imagine this is the preferred way
Would prefer if there was an e2e test with an invalid as well as valid 
`--authorization_factory_class`. These tests could be put in a new, separate 
e2e test file.



--
To view, visit http://gerrit.cloudera.org:8080/12901
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I001c20505ba4f0562b60fdef73d15308e8500c19
Gerrit-Change-Number: 12901
Gerrit-PatchSet: 4
Gerrit-Owner: radford nguyen <[email protected]>
Gerrit-Reviewer: Austin Nobis <[email protected]>
Gerrit-Reviewer: Fredy Wijaya <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Reviewer: radford nguyen <[email protected]>
Gerrit-Comment-Date: Thu, 11 Apr 2019 05:33:25 +0000
Gerrit-HasComments: Yes

Reply via email to