Skip to content

Commit 6b212f5

Browse files
LuisFigueroaGntkathole
authored andcommitted
fix: Refresh the SQL registry cache after deleting a permission
delete_permission ran its own DELETE and returned, so unlike the other delete_* methods it never bumped the project's last-updated metadata or refreshed the cache in sync mode. list_permissions(allow_cache=True) kept returning the deleted permission. Route it through _delete_object like the other object types. That alone wasn't enough: _delete_object refreshed the cache inside the open write transaction, so the refresh read a snapshot that still contained the deleted row. Refresh after the transaction commits instead, matching _apply_object. This also fixes stale cached reads after delete_entity, delete_data_source and the other deletes that go through _delete_object. Signed-off-by: LuisFigueroaG <[email protected]>
1 parent e2a53bb commit 6b212f5

2 files changed

Lines changed: 43 additions & 12 deletions

File tree

‎sdk/python/feast/infra/registry/sql.py‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1751,9 +1751,11 @@ def _delete_object(
17511751
if not self.purge_feast_metadata:
17521752
self._set_last_updated_metadata(_utc_now(), project, conn)
17531753

1754-
if self.cache_mode == "sync":
1755-
self.refresh()
1756-
return rows.rowcount
1754+
# Refresh only after the transaction commits, otherwise the cache is
1755+
# rebuilt from a snapshot that still contains the deleted object.
1756+
if self.cache_mode == "sync":
1757+
self.refresh()
1758+
return rows.rowcount
17571759

17581760
def _get_object(
17591761
self,
@@ -1927,14 +1929,9 @@ def apply_permission(
19271929
)
19281930

19291931
def delete_permission(self, name: str, project: str, commit: bool = True):
1930-
with self.write_engine.begin() as conn:
1931-
stmt = delete(permissions).where(
1932-
permissions.c.permission_name == name,
1933-
permissions.c.project_id == project,
1934-
)
1935-
rows = conn.execute(stmt)
1936-
if rows.rowcount < 1:
1937-
raise PermissionNotFoundException(name, project)
1932+
return self._delete_object(
1933+
permissions, name, project, "permission_name", PermissionNotFoundException
1934+
)
19381935

19391936
def _list_projects(
19401937
self,

‎sdk/python/tests/unit/infra/registry/test_sql_registry.py‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
from feast import Field
2626
from feast.data_source import PushSource
2727
from feast.entity import Entity
28-
from feast.errors import ConflictingFeatureViewNames
28+
from feast.errors import ConflictingFeatureViewNames, PermissionNotFoundException
2929
from feast.feature_view import FeatureView
3030
from feast.infra.offline_stores.file_source import FileSource
3131
from feast.infra.registry.sql import (
@@ -38,6 +38,9 @@
3838
from feast.infra.registry.sql import (
3939
metadata as registry_metadata,
4040
)
41+
from feast.permissions.action import AuthzedAction
42+
from feast.permissions.permission import Permission
43+
from feast.permissions.policy import RoleBasedPolicy
4144
from feast.protos.feast.core.Transformation_pb2 import (
4245
FeatureTransformationV2,
4346
UserDefinedFunctionV2,
@@ -347,6 +350,37 @@ def test_sql_registry(sqlite_registry):
347350
sqlite_registry.get_entity("test_entity", "test_project")
348351

349352

353+
def test_delete_permission_refreshes_cache(sqlite_registry):
354+
permission = Permission(
355+
name="reader",
356+
types=FeatureView,
357+
policy=RoleBasedPolicy(roles=["reader"]),
358+
actions=[AuthzedAction.DESCRIBE],
359+
)
360+
sqlite_registry.apply_permission(permission, "test_project")
361+
assert [
362+
p.name
363+
for p in sqlite_registry.list_permissions("test_project", allow_cache=True)
364+
] == ["reader"]
365+
366+
sqlite_registry.delete_permission("reader", "test_project")
367+
368+
assert sqlite_registry.list_permissions("test_project", allow_cache=True) == []
369+
with pytest.raises(PermissionNotFoundException):
370+
sqlite_registry.delete_permission("reader", "test_project")
371+
372+
373+
def test_delete_entity_refreshes_cache(sqlite_registry):
374+
sqlite_registry.apply_entity(Entity(name="driver"), "test_project")
375+
assert [
376+
e.name for e in sqlite_registry.list_entities("test_project", allow_cache=True)
377+
] == ["driver"]
378+
379+
sqlite_registry.delete_entity("driver", "test_project")
380+
381+
assert sqlite_registry.list_entities("test_project", allow_cache=True) == []
382+
383+
350384
def _build_feature_view(name: str, entity: Entity, source: FileSource) -> FeatureView:
351385
return FeatureView(
352386
name=name,

0 commit comments

Comments
 (0)