Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20916 )

Change subject: IMPALA-12578: Pass owner user of database and table to Ranger 
in GRANT/REVOKE
......................................................................


Patch Set 2: Code-Review+2

(2 comments)

Can be merged from my side, some optional ideas to further simplify test code

http://gerrit.cloudera.org:8080/#/c/20916/2/tests/authorization/test_ranger.py
File tests/authorization/test_ranger.py:

http://gerrit.cloudera.org:8080/#/c/20916/2/tests/authorization/test_ranger.py@1185
PS2, Line 1185: owner_user, admin_client, resource_owner_group,
              :                 err_grant, revoke_database_stmt, err_revoke, 
resource_owner_role)
Optional: lot of these parameters are just constants or come getuser(). These 
could be moved to global or class constanst to avoid passing so many arguments.


http://gerrit.cloudera.org:8080/#/c/20916/2/tests/authorization/test_ranger.py@1206
PS2, Line 1206:             # Revoke the privileges that were granted by 
'owner_user' in case any of the
              :             # REVOKE statements submitted by 'owner_user' 
failed to prevent this test
              :             # from interfering with other tests.
              :             admin_client.execute(revoke_database_stmt
              :                 .format(privilege, unique_database, 
grantee_type, grantee), user=ADMIN)
              :             # The CREATE privilege on a table is not supported.
              :             if privilege != "create":
              :               admin_client.execute(revoke_table_stmt
              :                   .format(privilege, unique_database, 
table_name, grantee_type, grantee),
              :                   user=ADMIN)
              :             # For a column, only the SELECT privilege is 
allowed.
              :             if privilege == "select":
              :               admin_client.execute(revoke_column_stmt
              :                   .format(privilege, column_names[0], 
unique_database, table_name,
              :                   grantee_type, grantee), user=ADMIN)
optional: Couldn't this cleanup code go into the individual functions, e.g. the 
revoke database one to _test_grant_revoke_by_owner_on_database? This would also 
allow moving revoke_database_stmt inside the function



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibac5335c65a860963ef0ccd890a926af80585ef3
Gerrit-Change-Number: 20916
Gerrit-PatchSet: 2
Gerrit-Owner: Fang-Yu Rao <[email protected]>
Gerrit-Reviewer: Aman Sinha <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Fang-Yu Rao <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Comment-Date: Wed, 31 Jan 2024 14:10:12 +0000
Gerrit-HasComments: Yes

Reply via email to