From 03e84b3135b04b5f4de2fb3f7c7fc81482c07b90 Mon Sep 17 00:00:00 2001 From: mayuriphad Date: Tue, 18 Aug 2026 17:22:34 +0530 Subject: [PATCH 1/2] fix: detect duplicate feature view/data source names at parse time Signed-off-by: mayuriphad --- sdk/python/feast/cli/cli.py | 6 +++++- sdk/python/feast/repo_operations.py | 20 +++++++++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/sdk/python/feast/cli/cli.py b/sdk/python/feast/cli/cli.py index 7826dfb650a..1fc98bbea19 100644 --- a/sdk/python/feast/cli/cli.py +++ b/sdk/python/feast/cli/cli.py @@ -52,7 +52,7 @@ from feast.cli.ui import ui from feast.cli.validation_references import validation_references_cmd from feast.constants import FEAST_FS_YAML_FILE_PATH_ENV_NAME -from feast.errors import FeastProviderLoginError +from feast.errors import FeastError, FeastProviderLoginError from feast.repo_config import load_repo_config from feast.repo_operations import ( apply_total, @@ -261,6 +261,8 @@ def plan_command( plan(repo_config, repo, skip_source_validation, skip_feature_view_validation) except FeastProviderLoginError as e: print(str(e)) + except FeastError as e: + raise click.ClickException(str(e)) @cli.command("apply", cls=NoOptionDefaultFormat) @@ -319,6 +321,8 @@ def apply_total_command( ) except FeastProviderLoginError as e: print(str(e)) + except FeastError as e: + raise click.ClickException(str(e)) @cli.command("teardown", cls=NoOptionDefaultFormat) diff --git a/sdk/python/feast/repo_operations.py b/sdk/python/feast/repo_operations.py index 02747ac4b54..afb83ca3414 100644 --- a/sdk/python/feast/repo_operations.py +++ b/sdk/python/feast/repo_operations.py @@ -22,7 +22,11 @@ from feast.diff.registry_diff import extract_objects_for_keep_delete_update_add from feast.entity import Entity from feast.feature_service import FeatureService -from feast.feature_store import FeatureStore +from feast.feature_store import ( + FeatureStore, + _validate_data_sources, + _validate_feature_views, +) from feast.feature_view import DUMMY_ENTITY, FeatureView from feast.file_utils import replace_str_in_file from feast.infra.registry.base_registry import BaseRegistry @@ -238,6 +242,20 @@ def parse_repo(repo_root: Path) -> RepoContents: res.projects.append(obj) res.entities.append(DUMMY_ENTITY) + + # Fail fast on duplicate feature view / data source names, before any + # heavy dependencies (FeatureStore, Dask, PySpark) are initialized. See + # https://github.com/feast-dev/feast/issues/6417 - detecting this later, + # inside store.plan()/store.apply(), risks the error being masked by a + # slow subprocess/atexit shutdown timing out before it can be reported. + _validate_feature_views( + res.feature_views + + res.on_demand_feature_views + + res.stream_feature_views + + res.label_views + ) + _validate_data_sources(res.data_sources) + return res From 7b913cecd7d0a336b5aaba4e649c38f8a2a9890b Mon Sep 17 00:00:00 2001 From: mayuriphad Date: Thu, 20 Aug 2026 17:49:26 +0530 Subject: [PATCH 2/2] fix: honor skip-feature-view-validation flag and avoid private cross-module imports Address review feedback on the duplicate feature view/data source name check added at parse time: - Move the fail-fast validation from parse_repo() (which has no access to CLI flags) into _get_repo_contents(), gated by skip_feature_view_validation so users who explicitly opt out via --skip-feature-view-validation are no longer blocked by it. - Thread skip_feature_view_validation through plan() and apply_total() to _get_repo_contents(), matching how store.plan()/store.apply() already honor the flag. - Rename _validate_feature_views/_validate_data_sources in feature_store.py to public validate_feature_views/validate_data_sources, since repo_operations.py now depends on them across module boundaries. Signed-off-by: mayuriphad --- sdk/python/feast/feature_store.py | 8 +-- sdk/python/feast/repo_operations.py | 49 ++++++++++++------- .../registration/test_feature_store.py | 18 +++---- sdk/python/tests/unit/test_label_view.py | 4 +- 4 files changed, 47 insertions(+), 32 deletions(-) diff --git a/sdk/python/feast/feature_store.py b/sdk/python/feast/feature_store.py index 546f6f7680d..ec42f88ff68 100644 --- a/sdk/python/feast/feature_store.py +++ b/sdk/python/feast/feature_store.py @@ -1134,7 +1134,7 @@ def _validate_all_feature_views( "This API is stable, but the functionality does not scale well for offline retrieval", RuntimeWarning, ) - _validate_feature_views( + validate_feature_views( [ *views_to_update, *odfvs_to_update, @@ -1425,7 +1425,7 @@ def plan( desired_repo_contents.stream_feature_views, desired_repo_contents.label_views, ) - _validate_data_sources(desired_repo_contents.data_sources) + validate_data_sources(desired_repo_contents.data_sources) self._make_inferences( desired_repo_contents.data_sources, desired_repo_contents.entities, @@ -5020,7 +5020,7 @@ def _print_materialization_log( ) -def _validate_feature_views(feature_views: List[BaseFeatureView]): +def validate_feature_views(feature_views: List[BaseFeatureView]): """Verify feature views have case-insensitively unique names across all types. This validates that no two feature views (of any type: FeatureView, @@ -5042,7 +5042,7 @@ def _validate_feature_views(feature_views: List[BaseFeatureView]): fv_by_name[case_insensitive_fv_name] = fv -def _validate_data_sources(data_sources: List[DataSource]): +def validate_data_sources(data_sources: List[DataSource]): """Verify data sources have case-insensitively unique names.""" ds_names = set() for ds in data_sources: diff --git a/sdk/python/feast/repo_operations.py b/sdk/python/feast/repo_operations.py index afb83ca3414..064666c9be9 100644 --- a/sdk/python/feast/repo_operations.py +++ b/sdk/python/feast/repo_operations.py @@ -24,8 +24,8 @@ from feast.feature_service import FeatureService from feast.feature_store import ( FeatureStore, - _validate_data_sources, - _validate_feature_views, + validate_data_sources, + validate_feature_views, ) from feast.feature_view import DUMMY_ENTITY, FeatureView from feast.file_utils import replace_str_in_file @@ -243,19 +243,6 @@ def parse_repo(repo_root: Path) -> RepoContents: res.entities.append(DUMMY_ENTITY) - # Fail fast on duplicate feature view / data source names, before any - # heavy dependencies (FeatureStore, Dask, PySpark) are initialized. See - # https://github.com/feast-dev/feast/issues/6417 - detecting this later, - # inside store.plan()/store.apply(), risks the error being masked by a - # slow subprocess/atexit shutdown timing out before it can be reported. - _validate_feature_views( - res.feature_views - + res.on_demand_feature_views - + res.stream_feature_views - + res.label_views - ) - _validate_data_sources(res.data_sources) - return res @@ -266,7 +253,12 @@ def plan( skip_feature_view_validation: bool = False, ): os.chdir(repo_path) - repo = _get_repo_contents(repo_path, repo_config.project, repo_config) + repo = _get_repo_contents( + repo_path, + repo_config.project, + repo_config, + skip_feature_view_validation=skip_feature_view_validation, + ) for project in repo.projects: repo_config.project = project.name store, registry = _get_store_and_registry(repo_config) @@ -291,10 +283,28 @@ def _get_repo_contents( repo_path, project_name: Optional[str] = None, repo_config: Optional[RepoConfig] = None, + skip_feature_view_validation: bool = False, ): sys.dont_write_bytecode = True repo = parse_repo(repo_path) + # Fail fast on duplicate feature view / data source names, before any + # heavy dependencies (FeatureStore, Dask, PySpark) are initialized. See + # https://github.com/feast-dev/feast/issues/6417 - detecting this later, + # inside store.plan()/store.apply(), risks the error being masked by a + # slow subprocess/atexit shutdown timing out before it can be reported. + # This mirrors the skip_feature_view_validation flag honored later in + # store.plan()/store.apply(), so users who pass + # --skip-feature-view-validation aren't blocked here either. + if not skip_feature_view_validation: + validate_feature_views( + repo.feature_views + + repo.on_demand_feature_views + + repo.stream_feature_views + + repo.label_views + ) + validate_data_sources(repo.data_sources) + if len(repo.projects) < 1: if project_name: print( @@ -531,7 +541,12 @@ def apply_total( no_promote: bool = False, ): os.chdir(repo_path) - repo = _get_repo_contents(repo_path, repo_config.project, repo_config) + repo = _get_repo_contents( + repo_path, + repo_config.project, + repo_config, + skip_feature_view_validation=skip_feature_view_validation, + ) for project in repo.projects: repo_config.project = project.name store, registry = _get_store_and_registry(repo_config) diff --git a/sdk/python/tests/integration/registration/test_feature_store.py b/sdk/python/tests/integration/registration/test_feature_store.py index 93334fbda19..d0747d7de3e 100644 --- a/sdk/python/tests/integration/registration/test_feature_store.py +++ b/sdk/python/tests/integration/registration/test_feature_store.py @@ -23,7 +23,7 @@ from feast.data_source import KafkaSource from feast.entity import Entity from feast.errors import ConflictingFeatureViewNames -from feast.feature_store import FeatureStore, _validate_feature_views +from feast.feature_store import FeatureStore, validate_feature_views from feast.feature_view import FeatureView from feast.field import Field from feast.infra.online_stores.sqlite import SqliteOnlineStoreConfig @@ -88,7 +88,7 @@ def feature_store_with_local_registry(): @pytest.mark.integration def test_validate_feature_views_cross_type_conflict(): """ - Test that _validate_feature_views() catches cross-type name conflicts. + Test that validate_feature_views() catches cross-type name conflicts. This is a unit test for the validation that happens during feast plan/apply. The validation must catch conflicts across FeatureView, StreamFeatureView, @@ -129,7 +129,7 @@ def test_validate_feature_views_cross_type_conflict(): # Validate should raise ConflictingFeatureViewNames with pytest.raises(ConflictingFeatureViewNames) as exc_info: - _validate_feature_views([feature_view, stream_feature_view]) + validate_feature_views([feature_view, stream_feature_view]) # Verify error message contains type information error_message = str(exc_info.value) @@ -140,7 +140,7 @@ def test_validate_feature_views_cross_type_conflict(): def test_validate_feature_views_same_type_conflict(): """ - Test that _validate_feature_views() also catches same-type name conflicts + Test that validate_feature_views() also catches same-type name conflicts with a proper error message indicating duplicate FeatureViews. """ # Create a simple entity @@ -163,7 +163,7 @@ def test_validate_feature_views_same_type_conflict(): # Validate should raise ConflictingFeatureViewNames with pytest.raises(ConflictingFeatureViewNames) as exc_info: - _validate_feature_views([fv1, fv2]) + validate_feature_views([fv1, fv2]) # Verify error message indicates same-type duplicate error_message = str(exc_info.value) @@ -174,7 +174,7 @@ def test_validate_feature_views_same_type_conflict(): def test_validate_feature_views_case_insensitive(): """ - Test that _validate_feature_views() catches case-insensitive conflicts. + Test that validate_feature_views() catches case-insensitive conflicts. """ entity = Entity(name="driver_entity", join_keys=["test_key"]) file_source = FileSource(name="my_file_source", path="test.parquet") @@ -194,12 +194,12 @@ def test_validate_feature_views_case_insensitive(): # Validate should raise ConflictingFeatureViewNames (case-insensitive) with pytest.raises(ConflictingFeatureViewNames): - _validate_feature_views([fv1, fv2]) + validate_feature_views([fv1, fv2]) def test_validate_feature_views_odfv_conflict(): """ - Test that _validate_feature_views() catches OnDemandFeatureView name conflicts. + Test that validate_feature_views() catches OnDemandFeatureView name conflicts. """ entity = Entity(name="driver_entity", join_keys=["test_key"]) file_source = FileSource(name="my_file_source", path="test.parquet") @@ -220,7 +220,7 @@ def shared_name(inputs: pd.DataFrame) -> pd.DataFrame: # Validate should raise ConflictingFeatureViewNames with pytest.raises(ConflictingFeatureViewNames) as exc_info: - _validate_feature_views([fv, shared_name]) + validate_feature_views([fv, shared_name]) error_message = str(exc_info.value) assert "shared_name" in error_message diff --git a/sdk/python/tests/unit/test_label_view.py b/sdk/python/tests/unit/test_label_view.py index f08aac72dbb..fcb1d095fd1 100644 --- a/sdk/python/tests/unit/test_label_view.py +++ b/sdk/python/tests/unit/test_label_view.py @@ -369,7 +369,7 @@ def test_label_view_in_feature_views_list_includes_type(self): assert not isinstance(lv, ODFV) def test_validate_feature_views_catches_name_conflict(self): - from feast.feature_store import _validate_feature_views + from feast.feature_store import validate_feature_views entity = Entity(name="item", join_keys=["item_id"], value_type=ValueType.STRING) lv1 = LabelView( @@ -391,7 +391,7 @@ def test_validate_feature_views_catches_name_conflict(self): from feast.errors import ConflictingFeatureViewNames with pytest.raises(ConflictingFeatureViewNames): - _validate_feature_views([lv1, lv2]) + validate_feature_views([lv1, lv2]) def test_materialization_task_accepts_label_view(self): from datetime import datetime