Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/22716 )
Change subject: IMPALA-13011: Support authorization for Calcite in Impala ...................................................................... Patch Set 22: (2 comments) In patch set 22, I have addressed Riza's comments on the patch set 21. Let me know if there is any additional suggestion. Thanks! http://gerrit.cloudera.org:8080/#/c/22716/21/tests/authorization/test_ranger.py File tests/authorization/test_ranger.py: http://gerrit.cloudera.org:8080/#/c/22716/21/tests/authorization/test_ranger.py@1425 PS21, Line 1425: with self.create_impala_client(user=ADMIN) as admin_client, \ : self.create_impala_client(user=grantee_user) as non_owner_cli > Always make sure custom clients are closed at the end of test method, regar Thanks for pointing this out Riza! I will use with statement in the next patch as you suggested in the next patch. I also noticed that in test_ranger.py we do not use the with statement when creating an Impala client/connection for most of the tests except for some tests under the class TestRangerIcebergRestCatalog, and we do not explicitly close the Impala client/connection either afterwards. I guess it's better we fix this in a follow-up JIRA? http://gerrit.cloudera.org:8080/#/c/22716/21/tests/authorization/test_ranger.py@2039 PS21, Line 2039: reset_ranger=True) : def test_no_exception_in_show_roles_if_no_roles_in_ranger(self): : self._test_no_exception_in_show_roles_if_no_roles_in_ranger() : : : @CustomClusterTestSuite.with_args( : impalad_args=LEGACY_CATALOG_IMPALAD_ARGS, : catalogd_args=LEGACY_CATALOG_CATALOGD_ARGS) : class TestRangerLegacyCatalog(TestRanger): : """ : Tests for Apache Ranger integration with Apache Impala in legacy catalog mode. : Test methods shares common cluster. : """ : : @pytest.mark.execute_serially : def test_grant_revoke_with_catalog_v1(self, unique_name): : """Tests grant/revoke with catalog v1.""" : self._test_grant_revoke(unique_name, [None, "invalidate metadata", : "refresh authorization"]) : : @pytest.mark.execute_serially : @SkipIfFS.hdfs_acls : def test_insert_with_catalog_v1(self, unique_name): : """ : Test that when Ranger is the authorization provider in the legacy catalog mode, : Impala does not throw an AnalysisException when an authorized user tries to execute : an INSERT query against a partitioned table of which the respective table path and : the partition path are not writable according to HDFS permission. : """ : user = getuser() : admin_client = self.create_impala_client(user=ADMIN) : unique_database = unique_name + "_db" : unique_table = unique_name + "_tbl" : partition_column = "year" : partition_value = "2008" : table_path = "test-warehouse/{0}.db/{1}".format(unique Not a problem. I will create the class TestRangerWithCalcite to contain these newly added tests and will focus on testing the integration in the local catalog mode. > I expect that we can drop this test once FE test + Calcite works. Yeah, we could probably drop some tests like test_select_calcite_frontend_with_local_catalog() or test_table_masking_calcite_frontend_with_local_catalog() after we could run the FE authorization tests against the Calcite planner. For test_view_on_view_all_configs_with_local_catalog(), since this test also verifies the behavior of Impala's classic frontend so we may still want to keep it somewhere after we could run the FE authorization tests against the Calcite planner. -- To view, visit http://gerrit.cloudera.org:8080/22716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9a7f7e4dc9a86a2da9e387832e552538e34029c1 Gerrit-Change-Number: 22716 Gerrit-PatchSet: 22 Gerrit-Owner: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Steve Carlin <[email protected]> Gerrit-Comment-Date: Mon, 01 Sep 2025 18:40:27 +0000 Gerrit-HasComments: Yes
