Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11888 )
Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement ...................................................................... Patch Set 14: Code-Review+2 (8 comments) just a bunch of nits, lgtm otherwise. http://gerrit.cloudera.org:8080/#/c/11888/14/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java: http://gerrit.cloudera.org:8080/#/c/11888/14/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1162 PS14, Line 1162: catalogs added nit: ..objects added.. (below too) http://gerrit.cloudera.org:8080/#/c/11888/14/fe/src/main/java/org/apache/impala/util/SentryProxy.java File fe/src/main/java/org/apache/impala/util/SentryProxy.java: http://gerrit.cloudera.org:8080/#/c/11888/14/fe/src/main/java/org/apache/impala/util/SentryProxy.java@75 PS14, Line 75: catalogs nit: catalog http://gerrit.cloudera.org:8080/#/c/11888/14/fe/src/main/java/org/apache/impala/util/SentryProxy.java@261 PS14, Line 261: } Log something here for debugging? Added x roles, removed y roles .. http://gerrit.cloudera.org:8080/#/c/11888/14/tests/authorization/test_authorization.py File tests/authorization/test_authorization.py: http://gerrit.cloudera.org:8080/#/c/11888/14/tests/authorization/test_authorization.py@524 PS14, Line 524: The Sentry long polling rephrase? http://gerrit.cloudera.org:8080/#/c/11888/14/tests/authorization/test_authorization.py@529 PS14, Line 529: 1 if sync_ddl else 0 nit: I think you can directly do: for sync_ddl in [0, 1]: opts = {'sync_ddl': sync_ddl} if sync_ddl: .... http://gerrit.cloudera.org:8080/#/c/11888/14/tests/authorization/test_authorization.py@550 PS14, Line 550: clients): add a docstring http://gerrit.cloudera.org:8080/#/c/11888/14/tests/authorization/test_authorization.py@591 PS14, Line 591: clients): doc string http://gerrit.cloudera.org:8080/#/c/11888/14/tests/authorization/test_authorization.py@606 PS14, Line 606: self.execute_query_expect_failure(self.client, : "select * from functional.alltypes limit 1") nit: should we loop this for all the clients? -- To view, visit http://gerrit.cloudera.org:8080/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 14 Gerrit-Owner: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Paul Rogers <prog...@cloudera.com> Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com> Gerrit-Comment-Date: Fri, 04 Jan 2019 00:49:07 +0000 Gerrit-HasComments: Yes
