Adam Holley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11837 )
Change subject: IMPALA-7794: Rewrite flaky ownership authorization tests ...................................................................... Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/11837/3/tests/authorization/test_grant_revoke.py File tests/authorization/test_grant_revoke.py: http://gerrit.cloudera.org:8080/#/c/11837/3/tests/authorization/test_grant_revoke.py@116 PS3, Line 116: "all" Could you "for privilege in ["all", "select"]"? http://gerrit.cloudera.org:8080/#/c/11837/3/tests/authorization/test_owner_privileges.py File tests/authorization/test_owner_privileges.py: http://gerrit.cloudera.org:8080/#/c/11837/3/tests/authorization/test_owner_privileges.py@97 PS3, Line 97: count count always seems to be 0 in usage. Any reason to keep that, or just rename to "validate_no_user_privileges"? http://gerrit.cloudera.org:8080/#/c/11837/3/tests/authorization/test_owner_privileges.py@97 PS3, Line 97: validate_user_privilege_count You can remove the "query" parameter, and just use the user to run "show grant user %s" % user. http://gerrit.cloudera.org:8080/#/c/11837/3/tests/common/sentry_cache_test_suite.py File tests/common/sentry_cache_test_suite.py: http://gerrit.cloudera.org:8080/#/c/11837/3/tests/common/sentry_cache_test_suite.py@40 PS3, Line 40: # We ignore the create_time column since it can be NULL. If it's NULL after invalidate, that would be a problem. http://gerrit.cloudera.org:8080/#/c/11837/3/tests/common/sentry_cache_test_suite.py@56 PS3, Line 56: validate_privileges You could get rid of the "query" parameter in this method and add an optional "for_role" parameter for the cases where you want to show role privileges. http://gerrit.cloudera.org:8080/#/c/11837/3/tests/common/sentry_cache_test_suite.py@68 PS3, Line 68: root user "specified client" - incorrect in original comment. -- To view, visit http://gerrit.cloudera.org:8080/11837 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic98f8dbec41360261fd0339d835f3ce6b504ee29 Gerrit-Change-Number: 11837 Gerrit-PatchSet: 3 Gerrit-Owner: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Adam Holley <ahol...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-Comment-Date: Wed, 31 Oct 2018 15:35:19 +0000 Gerrit-HasComments: Yes
