remove _apply_grants_config _revoke_grants_config API from engine adapter

and make sync_grants_config public

Author: newtonapple

sqlmesh/core/engine_adapter/base.py

@@ -2419,7 +2419,7 @@ def _get_current_grants_config(self, table: exp.Table) -> GrantsConfig:
             raise NotImplementedError(f"Engine does not support grants: {type(self)}")
         raise NotImplementedError("Subclass must implement get_current_grants")
 
-    def _sync_grants_config(
+    def sync_grants_config(
         self,
         table: exp.Table,
         grants_config: GrantsConfig,
@@ -2446,45 +2446,6 @@ def _sync_grants_config(
         if dcl_exprs:
             self.execute(dcl_exprs)
 
-    def _apply_grants_config(
-        self,
-        table: exp.Table,
-        grants_config: GrantsConfig,
-        table_type: DataObjectType = DataObjectType.TABLE,
-    ) -> None:
-        """Applies grants to a table.
-
-        Args:
-            table: The table/view to grant permissions on.
-            grants_config: Dictionary mapping privileges to lists of grantees.
-            table_type: The type of database object (TABLE, VIEW, MATERIALIZED_VIEW).
-
-        Raises:
-            NotImplementedError: If the engine does not support grants.
-        """
-
-        if grants := self._apply_grants_config_expr(table, grants_config, table_type):
-            self.execute(grants)
-
-    def _revoke_grants_config(
-        self,
-        table: exp.Table,
-        grants_config: GrantsConfig,
-        table_type: DataObjectType = DataObjectType.TABLE,
-    ) -> None:
-        """Revokes grants from a table.
-
-        Args:
-            table: The table/view to revoke privileges from.
-            grants_config: Dictionary mapping privileges to lists of grantees.
-            table_type: The type of database object (TABLE, VIEW, MATERIALIZED_VIEW).
-
-        Raises:
-            NotImplementedError: If the engine does not support grants.
-        """
-        if revokes := self._revoke_grants_config_expr(table, grants_config, table_type):
-            self.execute(revokes)
-
     def _apply_grants_config_expr(
         self,
         table: exp.Table,

sqlmesh/core/snapshot/evaluator.py

@@ -1699,7 +1699,7 @@ def _apply_grants(
         logger.info(f"Applying grants for model {model.name} to table {table_name}")
 
         try:
-            self.adapter._sync_grants_config(
+            self.adapter.sync_grants_config(
                 exp.to_table(table_name, dialect=model.dialect),
                 grants_config,
                 model.grants_table_type,

tests/core/engine_adapter/integration/test_integration_postgres.py

@@ -374,159 +374,6 @@ def _mutate_config(gateway: str, config: Config):
 # Grants Integration Tests
 
 
-def test_grants_apply_on_table(
-    engine_adapter: PostgresEngineAdapter, ctx: TestContext, config: Config
-):
-    with create_users(engine_adapter, "reader", "writer", "admin") as roles:
-        table = ctx.table("grants_test_table")
-        engine_adapter.create_table(
-            table, {"id": exp.DataType.build("INT"), "name": exp.DataType.build("VARCHAR(50)")}
-        )
-
-        engine_adapter.execute(f"INSERT INTO {table} VALUES (1, 'test')")
-
-        grants_config = {
-            "SELECT": [roles["reader"]["username"], roles["admin"]["username"]],
-            "INSERT": [roles["writer"]["username"], roles["admin"]["username"]],
-            "DELETE": [roles["admin"]["username"]],
-        }
-
-        engine_adapter._apply_grants_config(table, grants_config)
-
-        schema_name = table.db
-        for role_data in roles.values():
-            engine_adapter.execute(
-                f'GRANT USAGE ON SCHEMA "{schema_name}" TO "{role_data["username"]}"'
-            )
-
-        current_grants = engine_adapter._get_current_grants_config(table)
-
-        assert "SELECT" in current_grants
-        assert roles["reader"]["username"] in current_grants["SELECT"]
-        assert roles["admin"]["username"] in current_grants["SELECT"]
-
-        assert "INSERT" in current_grants
-        assert roles["writer"]["username"] in current_grants["INSERT"]
-        assert roles["admin"]["username"] in current_grants["INSERT"]
-
-        assert "DELETE" in current_grants
-        assert roles["admin"]["username"] in current_grants["DELETE"]
-
-        # Reader should be able to SELECT but not INSERT
-        with engine_adapter_for_role(roles["reader"], ctx, config) as reader_adapter:
-            result = reader_adapter.fetchone(f"SELECT COUNT(*) FROM {table}")
-            assert result == (1,), "Reader should be able to SELECT from table"
-
-        with engine_adapter_for_role(roles["reader"], ctx, config) as reader_adapter:
-            with pytest.raises(Exception):
-                reader_adapter.execute(f"INSERT INTO {table} VALUES (2, 'test2')")
-
-        # Writer should be able to INSERT but not SELECT
-        with engine_adapter_for_role(roles["writer"], ctx, config) as writer_adapter:
-            writer_adapter.execute(f"INSERT INTO {table} VALUES (3, 'test3')")
-
-        with engine_adapter_for_role(roles["writer"], ctx, config) as writer_adapter:
-            with pytest.raises(Exception):
-                writer_adapter.fetchone(f"SELECT COUNT(*) FROM {table}")
-
-        # Admin should be able to SELECT, INSERT, and DELETE
-        with engine_adapter_for_role(roles["admin"], ctx, config) as admin_adapter:
-            result = admin_adapter.fetchone(f"SELECT COUNT(*) FROM {table}")
-            assert result == (2,), "Admin should be able to SELECT from table"
-
-            admin_adapter.execute(f"INSERT INTO {table} VALUES (4, 'test4')")
-            admin_adapter.execute(f"DELETE FROM {table} WHERE id = 4")
-
-
-def test_grants_apply_on_view(
-    engine_adapter: PostgresEngineAdapter, ctx: TestContext, config: Config
-):
-    with create_users(engine_adapter, "reader", "admin") as roles:
-        base_table = ctx.table("grants_base_table")
-        engine_adapter.create_table(
-            base_table,
-            {"id": exp.DataType.build("INT"), "value": exp.DataType.build("VARCHAR(50)")},
-        )
-
-        view_table = ctx.table("grants_test_view")
-        engine_adapter.create_view(view_table, exp.select().from_(base_table))
-
-        # Grant schema access for authentication tests
-        test_schema = view_table.db
-        for role_credentials in roles.values():
-            engine_adapter.execute(
-                f'GRANT USAGE ON SCHEMA "{test_schema}" TO "{role_credentials["username"]}"'
-            )
-
-        grants_config = {"SELECT": [roles["reader"]["username"], roles["admin"]["username"]]}
-
-        engine_adapter._apply_grants_config(view_table, grants_config)
-
-        current_grants = engine_adapter._get_current_grants_config(view_table)
-        assert "SELECT" in current_grants
-        assert roles["reader"]["username"] in current_grants["SELECT"]
-        assert roles["admin"]["username"] in current_grants["SELECT"]
-
-        # Test actual authentication - reader should be able to SELECT from view
-        with engine_adapter_for_role(roles["reader"], ctx, config) as reader_adapter:
-            reader_adapter.fetchone(f"SELECT COUNT(*) FROM {view_table}")
-
-
-def test_grants_revoke(engine_adapter: PostgresEngineAdapter, ctx: TestContext, config: Config):
-    with create_users(engine_adapter, "reader", "writer") as roles:
-        table = ctx.table("grants_revoke_test")
-        engine_adapter.create_table(table, {"id": exp.DataType.build("INT")})
-        engine_adapter.execute(f"INSERT INTO {table} VALUES (1)")
-
-        # Grant schema access for authentication tests
-        test_schema = table.db
-        for role_credentials in roles.values():
-            engine_adapter.execute(
-                f'GRANT USAGE ON SCHEMA "{test_schema}" TO "{role_credentials["username"]}"'
-            )
-
-        initial_grants = {
-            "SELECT": [roles["reader"]["username"], roles["writer"]["username"]],
-            "INSERT": [roles["writer"]["username"]],
-        }
-        engine_adapter._apply_grants_config(table, initial_grants)
-
-        initial_current_grants = engine_adapter._get_current_grants_config(table)
-        assert roles["reader"]["username"] in initial_current_grants.get("SELECT", [])
-        assert roles["writer"]["username"] in initial_current_grants.get("SELECT", [])
-        assert roles["writer"]["username"] in initial_current_grants.get("INSERT", [])
-
-        # Verify reader can SELECT before revoke
-        with engine_adapter_for_role(roles["reader"], ctx, config) as reader_adapter:
-            reader_adapter.fetchone(f"SELECT COUNT(*) FROM {table}")
-
-        revoke_grants = {
-            "SELECT": [roles["reader"]["username"]],
-            "INSERT": [roles["writer"]["username"]],
-        }
-        engine_adapter._revoke_grants_config(table, revoke_grants)
-
-        current_grants_after = engine_adapter._get_current_grants_config(table)
-
-        assert roles["reader"]["username"] not in current_grants_after.get("SELECT", [])
-        assert roles["writer"]["username"] in current_grants_after.get("SELECT", [])
-        assert roles["writer"]["username"] not in current_grants_after.get("INSERT", [])
-
-        # Verify reader can NO LONGER SELECT after revoke
-        with engine_adapter_for_role(roles["reader"], ctx, config) as reader_adapter:
-            with pytest.raises(Exception):
-                reader_adapter.fetchone(f"SELECT COUNT(*) FROM {table}")
-
-        # Verify writer can still SELECT but not INSERT after revoke
-        with engine_adapter_for_role(roles["writer"], ctx, config) as writer_adapter:
-            result = writer_adapter.fetchone(f"SELECT COUNT(*) FROM {table}")
-            assert result is not None
-            assert result[0] == 1
-        with engine_adapter_for_role(roles["writer"], ctx, config) as writer_adapter:
-            with pytest.raises(Exception):
-                writer_adapter.execute(f"INSERT INTO {table} VALUES (2)")
-
-
 def test_grants_sync(engine_adapter: PostgresEngineAdapter, ctx: TestContext, config: Config):
     with create_users(engine_adapter, "user1", "user2", "user3") as roles:
         table = ctx.table("grants_sync_test")
@@ -538,7 +385,7 @@ def test_grants_sync(engine_adapter: PostgresEngineAdapter, ctx: TestContext, co
             "SELECT": [roles["user1"]["username"], roles["user2"]["username"]],
             "INSERT": [roles["user1"]["username"]],
         }
-        engine_adapter._apply_grants_config(table, initial_grants)
+        engine_adapter.sync_grants_config(table, initial_grants)
 
         initial_current_grants = engine_adapter._get_current_grants_config(table)
         assert roles["user1"]["username"] in initial_current_grants.get("SELECT", [])
@@ -549,7 +396,7 @@ def test_grants_sync(engine_adapter: PostgresEngineAdapter, ctx: TestContext, co
             "SELECT": [roles["user2"]["username"], roles["user3"]["username"]],
             "UPDATE": [roles["user3"]["username"]],
         }
-        engine_adapter._sync_grants_config(table, target_grants)
+        engine_adapter.sync_grants_config(table, target_grants)
 
         final_grants = engine_adapter._get_current_grants_config(table)
 
@@ -572,13 +419,13 @@ def test_grants_sync_empty_config(
             "SELECT": [roles["user"]["username"]],
             "INSERT": [roles["user"]["username"]],
         }
-        engine_adapter._apply_grants_config(table, initial_grants)
+        engine_adapter.sync_grants_config(table, initial_grants)
 
         initial_current_grants = engine_adapter._get_current_grants_config(table)
         assert roles["user"]["username"] in initial_current_grants.get("SELECT", [])
         assert roles["user"]["username"] in initial_current_grants.get("INSERT", [])
 
-        engine_adapter._sync_grants_config(table, {})
+        engine_adapter.sync_grants_config(table, {})
 
         final_grants = engine_adapter._get_current_grants_config(table)
         assert final_grants == {}
@@ -601,7 +448,7 @@ def test_grants_case_insensitive_grantees(
         writer = roles["test_writer"]["username"]
 
         grants_config = {"SELECT": [reader, writer.upper()]}
-        engine_adapter._apply_grants_config(table, grants_config)
+        engine_adapter.sync_grants_config(table, grants_config)
 
         # Grantees are still in lowercase
         current_grants = engine_adapter._get_current_grants_config(table)
@@ -610,7 +457,7 @@ def test_grants_case_insensitive_grantees(
 
         # Revoke writer
         grants_config = {"SELECT": [reader.upper()]}
-        engine_adapter._sync_grants_config(table, grants_config)
+        engine_adapter.sync_grants_config(table, grants_config)
 
         current_grants = engine_adapter._get_current_grants_config(table)
         assert reader in current_grants.get("SELECT", [])

tests/core/engine_adapter/test_base.py

@@ -3791,25 +3791,7 @@ def test_sync_grants_config_unsupported_engine(make_mocked_engine_adapter: t.Cal
     grants_config = {"SELECT": ["user1"]}
 
     with pytest.raises(NotImplementedError, match="Engine does not support grants"):
-        adapter._sync_grants_config(relation, grants_config)
-
-
-def test_apply_grants_config_expr_not_implemented(make_mocked_engine_adapter: t.Callable):
-    adapter = make_mocked_engine_adapter(EngineAdapter)
-    relation = exp.to_table("test_table")
-    grants_config = {"SELECT": ["user1"]}
-
-    with pytest.raises(NotImplementedError):
-        adapter._apply_grants_config_expr(relation, grants_config)
-
-
-def test_revoke_grants_config_expr_not_implemented(make_mocked_engine_adapter: t.Callable):
-    adapter = make_mocked_engine_adapter(EngineAdapter)
-    relation = exp.to_table("test_table")
-    grants_config = {"SELECT": ["user1"]}
-
-    with pytest.raises(NotImplementedError):
-        adapter._revoke_grants_config_expr(relation, grants_config)
+        adapter.sync_grants_config(relation, grants_config)
 
 
 def test_get_current_grants_config_not_implemented(make_mocked_engine_adapter: t.Callable):

tests/core/engine_adapter/test_postgres.py

@@ -7,7 +7,6 @@
 from sqlglot.helper import ensure_list
 
 from sqlmesh.core.engine_adapter import PostgresEngineAdapter
-from sqlmesh.core.engine_adapter.shared import DataObjectType
 from sqlmesh.utils.errors import SQLMeshError
 from tests.core.engine_adapter import to_sql_calls
 
@@ -180,34 +179,6 @@ def test_server_version(make_mocked_engine_adapter: t.Callable, mocker: MockerFi
     assert adapter.server_version == (15, 13)
 
 
-def test_apply_grants_config(make_mocked_engine_adapter: t.Callable):
-    adapter = make_mocked_engine_adapter(PostgresEngineAdapter)
-    relation = exp.to_table("test_table", dialect="postgres")
-    grants_config = {"SELECT": ["user1", "user2"], "INSERT": ["admin_user"]}
-
-    adapter._apply_grants_config(relation, grants_config, DataObjectType.TABLE)
-
-    sql_calls = to_sql_calls(adapter)
-
-    assert len(sql_calls) == 2
-    assert 'GRANT SELECT ON "test_table" TO user1, user2' in sql_calls
-    assert 'GRANT INSERT ON "test_table" TO admin_user' in sql_calls
-
-
-def test_revoke_grants_config(make_mocked_engine_adapter: t.Callable):
-    adapter = make_mocked_engine_adapter(PostgresEngineAdapter)
-    relation = exp.to_table("test_table", dialect="postgres")
-    grants_config = {"SELECT": ["old_user"], "INSERT": ["removed_role"]}
-
-    adapter._revoke_grants_config(relation, grants_config, DataObjectType.TABLE)
-
-    sql_calls = to_sql_calls(adapter)
-
-    assert len(sql_calls) == 2
-    assert 'REVOKE SELECT ON "test_table" FROM old_user' in sql_calls
-    assert 'REVOKE INSERT ON "test_table" FROM removed_role' in sql_calls
-
-
 def test_sync_grants_config(make_mocked_engine_adapter: t.Callable, mocker: MockerFixture):
     adapter = make_mocked_engine_adapter(PostgresEngineAdapter)
     relation = exp.to_table("test_schema.test_table", dialect="postgres")
@@ -216,7 +187,7 @@ def test_sync_grants_config(make_mocked_engine_adapter: t.Callable, mocker: Mock
     current_grants = [("SELECT", "old_user"), ("UPDATE", "admin_user")]
     fetchall_mock = mocker.patch.object(adapter, "fetchall", return_value=current_grants)
 
-    adapter._sync_grants_config(relation, new_grants_config)
+    adapter.sync_grants_config(relation, new_grants_config)
 
     fetchall_mock.assert_called_once()
     executed_query = fetchall_mock.call_args[0][0]
@@ -252,7 +223,7 @@ def test_sync_grants_config_with_overlaps(
     ]
     fetchall_mock = mocker.patch.object(adapter, "fetchall", return_value=current_grants)
 
-    adapter._sync_grants_config(relation, new_grants_config)
+    adapter.sync_grants_config(relation, new_grants_config)
 
     fetchall_mock.assert_called_once()
     executed_query = fetchall_mock.call_args[0][0]
@@ -298,7 +269,7 @@ def test_sync_grants_config_with_default_schema(
     fetchall_mock = mocker.patch.object(adapter, "fetchall", return_value=currrent_grants)
     get_schema_mock = mocker.patch.object(adapter, "get_current_schema", return_value="public")
 
-    adapter._sync_grants_config(relation, new_grants_config)
+    adapter.sync_grants_config(relation, new_grants_config)
 
     get_schema_mock.assert_called_once()