radford nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/12901 )
Change subject: IMPALA-8309: add user authorization_provider flag ...................................................................... Patch Set 2: (10 comments) > Patch Set 1: > > (8 comments) http://gerrit.cloudera.org:8080/#/c/12901/2/fe/src/main/java/org/apache/impala/service/BackendConfig.java File fe/src/main/java/org/apache/impala/service/BackendConfig.java: http://gerrit.cloudera.org:8080/#/c/12901/2/fe/src/main/java/org/apache/impala/service/BackendConfig.java@141 PS2, Line 141: public @Nullable String getAuthorizationFactoryClassOrNull() { Not sure if there's a standard for handling optional flags... at least I didn't really see one. We could alternatively return an `Optional` (generally my preference) or just return the raw value that the be populates and let callers decide what is "present" or not. http://gerrit.cloudera.org:8080/#/c/12901/1/fe/src/main/java/org/apache/impala/service/JniCatalog.java File fe/src/main/java/org/apache/impala/service/JniCatalog.java: http://gerrit.cloudera.org:8080/#/c/12901/1/fe/src/main/java/org/apache/impala/service/JniCatalog.java@113 PS1, Line 113: final AuthorizationFactory authzFactory > line too long (99 > 90) Ack http://gerrit.cloudera.org:8080/#/c/12901/1/fe/src/test/java/org/apache/impala/service/JniFrontendTest.java File fe/src/test/java/org/apache/impala/service/JniFrontendTest.java: http://gerrit.cloudera.org:8080/#/c/12901/1/fe/src/test/java/org/apache/impala/service/JniFrontendTest.java@92 PS1, Line 92: Class<? extends AuthorizationFactory> authorization_factory_class, > line too long (105 > 90) Ack http://gerrit.cloudera.org:8080/#/c/12901/1/fe/src/test/java/org/apache/impala/service/JniFrontendTest.java@93 PS1, Line 93: String authorization_provider) { > line too long (91 > 90) Ack http://gerrit.cloudera.org:8080/#/c/12901/1/fe/src/test/java/org/apache/impala/service/JniFrontendTest.java@110 PS1, Line 110: JniFrontend.authzFactoryClassNameFrom(authCfg("", "ranger")) > line too long (102 > 90) Ack http://gerrit.cloudera.org:8080/#/c/12901/1/fe/src/test/java/org/apache/impala/service/JniFrontendTest.java@113 PS1, Line 113: > line too long (98 > 90) Ack http://gerrit.cloudera.org:8080/#/c/12901/1/fe/src/test/java/org/apache/impala/service/JniFrontendTest.java@118 PS1, Line 118: SentryAuthorizationFactory.class.getCanonicalName(), > line too long (98 > 90) Ack http://gerrit.cloudera.org:8080/#/c/12901/1/fe/src/test/java/org/apache/impala/service/JniFrontendTest.java@123 PS1, Line 123: > line too long (98 > 90) Ack http://gerrit.cloudera.org:8080/#/c/12901/1/fe/src/test/java/org/apache/impala/service/JniFrontendTest.java@128 PS1, Line 128: ) > line too long (98 > 90) Ack http://gerrit.cloudera.org:8080/#/c/12901/2/fe/src/test/java/org/apache/impala/service/JniFrontendTest.java File fe/src/test/java/org/apache/impala/service/JniFrontendTest.java: http://gerrit.cloudera.org:8080/#/c/12901/2/fe/src/test/java/org/apache/impala/service/JniFrontendTest.java@47 PS2, Line 47: origFlags = BackendConfig.INSTANCE.getBackendCfg(); This seems to be what other tests do -- 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: 2 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: Mon, 01 Apr 2019 09:38:38 +0000 Gerrit-HasComments: Yes
