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/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 02747ac4b54..064666c9be9 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,7 @@ def parse_repo(repo_root: Path) -> RepoContents: res.projects.append(obj) res.entities.append(DUMMY_ENTITY) + return res @@ -248,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) @@ -273,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( @@ -513,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