Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12962 )
Change subject: [IMPALA-8227] Add support for WITH GRANT OPTION with Ranger ...................................................................... Patch Set 2: (8 comments) http://gerrit.cloudera.org:8080/#/c/12962/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12962/2//COMMIT_MSG@7 PS2, Line 7: [IMPALA-8227] nit: use IMPALA-8227: instead http://gerrit.cloudera.org:8080/#/c/12962/2/tests/authorization/test_ranger.py File tests/authorization/test_ranger.py: http://gerrit.cloudera.org:8080/#/c/12962/2/tests/authorization/test_ranger.py@119 PS2, Line 119: time.sleep(35) nit: put # TODO: IMPALA-8293 use refresh authorization http://gerrit.cloudera.org:8080/#/c/12962/2/tests/authorization/test_ranger.py@119 PS2, Line 119: time.sleep(35) : self.execute_query_expect_success(user1_client, : "grant select on database {0} to user {1}" : .format(unique_database, user2), user=user1) : # TODO: IMPALA-8293 use refresh authorization : time.sleep(35) : self.execute_query_expect_success(user2_client, "show tables in {0}" : .format(unique_database), user=user2) : self.execute_query_expect_success(user1_client, : "revoke select on database {0} from user " : "{1}".format(unique_database, user2), user=user1) : # TODO: IMPALA-8293 use refresh authorization : time.sleep(35) shouldn't all these sleeps be 10 seconds? http://gerrit.cloudera.org:8080/#/c/12962/2/tests/authorization/test_ranger.py@132 PS2, Line 132: self.execute_query_expect_failure(user2_client, "show tables in {0}" : .format(unique_database)) can we have a test case for "revoke grant option" here and verify it? http://gerrit.cloudera.org:8080/#/c/12962/2/tests/authorization/test_ranger.py@147 PS2, Line 147: http://localhost:6080 make the ranger admin host as a global variable, i.e. RANGER_ADMIN_HOST http://gerrit.cloudera.org:8080/#/c/12962/2/tests/authorization/test_ranger.py@148 PS2, Line 148: auth=("admin", "admin") nit: make this a global variable so that it's available to other functions, i.e. RANGER_AUTH http://gerrit.cloudera.org:8080/#/c/12962/2/tests/authorization/test_ranger.py@148 PS2, Line 148: auth=("admin", "admin"), : json=data, headers=headers) nit: fix indentation http://gerrit.cloudera.org:8080/#/c/12962/2/tests/authorization/test_ranger.py@143 PS2, Line 143: def _add_ranger_user(self, user): : data = {"name": user, "password": "password123", "userRoleList": ["ROLE_USER"]} : headers = {"Content-Type": "application/json", "Accept": "application/json"} : : r = requests.post("http://localhost:6080/service/xusers/secure/users";, : auth=("admin", "admin"), : json=data, headers=headers) : return json.loads(r.content)['id'] : : def _remove_ranger_user(self, id): : requests.delete("http://localhost:6080/service/xusers/users/{0}?forceDelete=true"; : .format(id), auth=("admin", "admin")) we should check for status code and return an error when it's not 2XX. -- To view, visit http://gerrit.cloudera.org:8080/12962 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9c2384f0a9fe30bea1eaceac5b27b1c432383aa8 Gerrit-Change-Number: 12962 Gerrit-PatchSet: 2 Gerrit-Owner: Austin Nobis <[email protected]> Gerrit-Reviewer: Austin Nobis <[email protected]> Gerrit-Reviewer: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Comment-Date: Tue, 09 Apr 2019 17:35:13 +0000 Gerrit-HasComments: Yes
