Skip to content

Commit 96562c4

Browse files
mailtoboggavarapu-codermailtoboggavarapu-coder
authored
fix: Add project filter to apply_data_source and delete_data_source (closes #6206) (#6322)
* fix: Add project filter to apply_data_source and delete_data_source (fixes #6206) Filters existing data sources by both name and project in the file registry implementation, fixing cross-project contamination (issue #6206). Changes: - apply_data_source: added project-scoped deduplication check - delete_data_source: added project filter to avoid cross-project deletion Signed-off-by: Venkateswarlu Boggavarapu <[email protected]> * test: add cross-project isolation tests for apply/delete_data_source Regression tests for #6206: - test_apply_data_source_cross_project_isolation: verifies applying a data source to project_a does not overwrite the same-named source in project_b - test_delete_data_source_project_scoped: verifies delete_data_source only removes the source from the specified project Signed-off-by: Venkateswarlu Boggavarapu <[email protected]> * style: remove extra blank line between decorator and function Signed-off-by: Venkateswarlu Boggavarapu <[email protected]> * fix(tests): scope cross-project isolation tests to local_registry only Changed parametrize decorators from all_fixtures to [lazy_fixture("local_registry")] for both new test functions to avoid polluting the session-scoped sqlite_registry fixture. Signed-off-by: Venkateswarlu Boggavarapu <[email protected]> --------- Signed-off-by: Venkateswarlu Boggavarapu <[email protected]>
1 parent 567dfe5 commit 96562c4

2 files changed

Lines changed: 116 additions & 2 deletions

File tree

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,12 @@ def list_data_sources(
386386
def apply_data_source(
387387
self, data_source: DataSource, project: str, commit: bool = True
388388
):
389+
"""Apply a data source to the registry with project-scoped deduplication.
390+
391+
Filters existing data sources by both name and project (fixes feast-dev/feast#6206),
392+
preserving the original created_timestamp if the source already exists in the
393+
target project.
394+
"""
389395
now = _utc_now()
390396
if not data_source.created_timestamp:
391397
data_source.created_timestamp = now
@@ -394,7 +400,10 @@ def apply_data_source(
394400
registry = self._prepare_registry_for_changes(project)
395401

396402
for idx, existing_data_source_proto in enumerate(registry.data_sources):
397-
if existing_data_source_proto.name == data_source.name:
403+
if (
404+
existing_data_source_proto.name == data_source.name
405+
and existing_data_source_proto.project == project
406+
):
398407
existing_data_source = DataSource.from_proto(existing_data_source_proto)
399408
# Check if the data source has actually changed
400409
if existing_data_source == data_source:
@@ -423,7 +432,7 @@ def delete_data_source(self, name: str, project: str, commit: bool = True):
423432
for idx, data_source_proto in enumerate(
424433
self.cached_registry_proto.data_sources
425434
):
426-
if data_source_proto.name == name:
435+
if data_source_proto.name == name and data_source_proto.project == project:
427436
del self.cached_registry_proto.data_sources[idx]
428437
if commit:
429438
self.commit()

‎sdk/python/tests/integration/registration/test_universal_registry.py‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -899,6 +899,111 @@ def test_apply_data_source_with_timestamps(test_registry):
899899
test_registry.teardown()
900900

901901

902+
@pytest.mark.integration
903+
@pytest.mark.parametrize(
904+
"test_registry",
905+
[lazy_fixture("local_registry")],
906+
)
907+
def test_apply_data_source_cross_project_isolation(test_registry):
908+
"""Test that apply_data_source uses project-scoped filtering.
909+
910+
Regression test for https://github.com/feast-dev/feast/issues/6206:
911+
applying a data source to one project must not overwrite the data source
912+
with the same name in a different project.
913+
914+
See: feast-dev/feast#6298
915+
"""
916+
project_a = "project_a"
917+
project_b = "project_b"
918+
919+
source_a = FileSource(
920+
name="shared_source_name",
921+
file_format=ParquetFormat(),
922+
path="file://feast/project_a.parquet",
923+
timestamp_field="ts_col",
924+
)
925+
source_b = FileSource(
926+
name="shared_source_name",
927+
file_format=ParquetFormat(),
928+
path="file://feast/project_b.parquet",
929+
timestamp_field="ts_col",
930+
)
931+
932+
test_registry.apply_data_source(source_a, project_a, commit=True)
933+
test_registry.apply_data_source(source_b, project_b, commit=True)
934+
935+
# Each project should have exactly its own source
936+
sources_a = test_registry.list_data_sources(project_a)
937+
sources_b = test_registry.list_data_sources(project_b)
938+
assert len(sources_a) == 1
939+
assert len(sources_b) == 1
940+
941+
# Paths must be project-specific — not overwritten cross-project
942+
assert sources_a[0].path == "file://feast/project_a.parquet"
943+
assert sources_b[0].path == "file://feast/project_b.parquet"
944+
945+
# Re-apply source_b with updated path: must not bleed into project_a
946+
source_b_updated = FileSource(
947+
name="shared_source_name",
948+
file_format=ParquetFormat(),
949+
path="file://feast/project_b_v2.parquet",
950+
timestamp_field="ts_col",
951+
)
952+
test_registry.apply_data_source(source_b_updated, project_b, commit=True)
953+
954+
sources_a_after = test_registry.list_data_sources(project_a)
955+
assert len(sources_a_after) == 1
956+
assert sources_a_after[0].path == "file://feast/project_a.parquet", (
957+
"apply_data_source for project_b must not overwrite project_a's source"
958+
)
959+
960+
test_registry.teardown()
961+
962+
963+
@pytest.mark.integration
964+
@pytest.mark.parametrize(
965+
"test_registry",
966+
[lazy_fixture("local_registry")],
967+
)
968+
def test_delete_data_source_project_scoped(test_registry):
969+
"""Test that delete_data_source only removes the source from the given project.
970+
971+
Regression test for https://github.com/feast-dev/feast/issues/6206:
972+
deleting a data source from one project must not delete the data source
973+
with the same name from another project.
974+
"""
975+
project_a = "project_a"
976+
project_b = "project_b"
977+
978+
source_a = FileSource(
979+
name="shared_source_name",
980+
file_format=ParquetFormat(),
981+
path="file://feast/project_a.parquet",
982+
timestamp_field="ts_col",
983+
)
984+
source_b = FileSource(
985+
name="shared_source_name",
986+
file_format=ParquetFormat(),
987+
path="file://feast/project_b.parquet",
988+
timestamp_field="ts_col",
989+
)
990+
991+
test_registry.apply_data_source(source_a, project_a, commit=True)
992+
test_registry.apply_data_source(source_b, project_b, commit=True)
993+
994+
# Delete the source from project_a only
995+
test_registry.delete_data_source("shared_source_name", project_a, commit=True)
996+
997+
# project_a should have no sources; project_b should be unaffected
998+
sources_a = test_registry.list_data_sources(project_a)
999+
sources_b = test_registry.list_data_sources(project_b)
1000+
assert len(sources_a) == 0, "Source should be deleted from project_a"
1001+
assert len(sources_b) == 1, "Source in project_b must not be deleted"
1002+
assert sources_b[0].path == "file://feast/project_b.parquet"
1003+
1004+
test_registry.teardown()
1005+
1006+
9021007
@pytest.mark.integration
9031008
@pytest.mark.parametrize(
9041009
"test_registry",

0 commit comments

Comments
 (0)