diff --git a/sdk/python/feast/errors.py b/sdk/python/feast/errors.py index 515a6c39b11..e9e02048fa5 100644 --- a/sdk/python/feast/errors.py +++ b/sdk/python/feast/errors.py @@ -507,6 +507,24 @@ def __init__(self): ) +class MissingJoinKeyValuesException(FeastError, KeyError): + """A required join key was not supplied for the requested feature views. + + Subclasses ``KeyError`` as well, because that is what this condition raised + before it carried an HTTP status, and callers catch it. + """ + + def __init__(self, missing_keys, empty_keys, provided_keys): + super().__init__( + f"Missing join key values for keys: {sorted(missing_keys)}. " + f"No values provided for keys: {sorted(empty_keys)}. " + f"Provided join_key_values: {list(provided_keys)}" + ) + + def http_status_code(self) -> int: + return HttpStatusCode.HTTP_400_BAD_REQUEST + + class PushSourceNotFoundException(FeastError): def __init__(self, push_source_name: str): super().__init__(f"Unable to find push source '{push_source_name}'.") diff --git a/sdk/python/feast/infra/online_stores/online_store.py b/sdk/python/feast/infra/online_stores/online_store.py index cdf06639fe0..b753a18d0bf 100644 --- a/sdk/python/feast/infra/online_stores/online_store.py +++ b/sdk/python/feast/infra/online_stores/online_store.py @@ -166,7 +166,10 @@ def get_online_features( include_feature_view_version_metadata: bool = False, ) -> OnlineResponse: if isinstance(entity_rows, list): - columnar: Dict[str, List[Any]] = {k: [] for k in entity_rows[0].keys()} + # An empty batch carries no column names; downstream handles zero rows. + columnar: Dict[str, List[Any]] = ( + {k: [] for k in entity_rows[0].keys()} if entity_rows else {} + ) for entity_row in entity_rows: for key, value in entity_row.items(): try: @@ -450,7 +453,10 @@ async def get_online_features_async( include_feature_view_version_metadata: bool = False, ) -> OnlineResponse: if isinstance(entity_rows, list): - columnar: Dict[str, List[Any]] = {k: [] for k in entity_rows[0].keys()} + # An empty batch carries no column names; downstream handles zero rows. + columnar: Dict[str, List[Any]] = ( + {k: [] for k in entity_rows[0].keys()} if entity_rows else {} + ) for entity_row in entity_rows: for key, value in entity_row.items(): try: diff --git a/sdk/python/feast/utils.py b/sdk/python/feast/utils.py index 4f8d18ad080..dd0d38a0d66 100644 --- a/sdk/python/feast/utils.py +++ b/sdk/python/feast/utils.py @@ -33,6 +33,7 @@ from feast.errors import ( FeatureNameCollisionError, FeatureViewNotFoundException, + MissingJoinKeyValuesException, RequestDataNotFoundInEntityRowsException, ) from feast.field import Field @@ -556,7 +557,8 @@ def _validate_entity_values(join_key_values: Dict[str, List[ValueProto]]): set_of_row_lengths = {len(v) for v in join_key_values.values()} if len(set_of_row_lengths) > 1: raise ValueError("All entity rows must have the same columns.") - return set_of_row_lengths.pop() + # No columns at all means no rows; popping an empty set would raise here. + return set_of_row_lengths.pop() if set_of_row_lengths else 0 def _validate_feature_refs(feature_refs: List[str], full_feature_names: bool = False): @@ -1025,11 +1027,13 @@ def _get_unique_entities( ) if missing_keys or empty_keys: - if not any(table_entity_values.values()): - raise KeyError( - f"Missing join key values for keys: {missing_keys}. " - f"No values provided for keys: {empty_keys}. " - f"Provided join_key_values: {list(join_key_values.keys())}" + # Columns present but all empty is a well-formed request that simply has + # no rows this run; the row-wise conversion below already returns empty + # results for it. Only a caller that supplied nothing at all for this + # view is reporting an error. + if not table_entity_values: + raise MissingJoinKeyValuesException( + missing_keys, empty_keys, join_key_values.keys() ) # Convert the column-oriented table_entity_values into row-wise data. diff --git a/sdk/python/tests/unit/online_store/test_empty_entity_rows.py b/sdk/python/tests/unit/online_store/test_empty_entity_rows.py new file mode 100644 index 00000000000..f9d08dd95c2 --- /dev/null +++ b/sdk/python/tests/unit/online_store/test_empty_entity_rows.py @@ -0,0 +1,73 @@ +"""An empty batch of entities must not surface internal errors. + +A caller whose upstream query matched nothing sends zero rows. That used to hit +``IndexError: list index out of range`` or ``KeyError: 'pop from an empty set'``, +which the feature server turned into a 500. +""" + +import asyncio + +import pytest +from fastapi import status as HttpStatusCode + +from feast.errors import MissingJoinKeyValuesException +from feast.utils import _validate_entity_values +from tests.utils.cli_repo_creator import CliRunner, get_example_repo + +FEATURES = ["driver_locations:lat", "driver_locations:lon"] + + +def test_validate_entity_values_treats_no_columns_as_zero_rows(): + """Popping the empty set raised instead of reporting zero rows.""" + assert _validate_entity_values({}) == 0 + + +def test_missing_join_key_values_is_a_client_error(): + exc = MissingJoinKeyValuesException(["driver_id"], [], []) + # Callers caught KeyError before this carried a status; keep that working. + assert isinstance(exc, KeyError) + assert exc.http_status_code() == HttpStatusCode.HTTP_400_BAD_REQUEST + + +def test_join_key_present_with_zero_values_returns_an_empty_response(): + """A well-formed request that simply has no rows this run.""" + runner = CliRunner() + with runner.local_repo( + get_example_repo("example_feature_repo_1.py"), "file" + ) as store: + response = store.get_online_features( + features=FEATURES, entity_rows={"driver_id": []} + ).to_dict() + + assert set(response.keys()) == {"driver_id", "lat", "lon"} + assert all(values == [] for values in response.values()) + + +@pytest.mark.parametrize("entity_rows", [[], {}], ids=["empty_list", "no_columns"]) +def test_request_without_join_keys_raises_a_client_error(entity_rows): + """No column names at all: report the missing join key, not an internal error.""" + runner = CliRunner() + with runner.local_repo( + get_example_repo("example_feature_repo_1.py"), "file" + ) as store: + with pytest.raises(MissingJoinKeyValuesException) as excinfo: + store.get_online_features(features=FEATURES, entity_rows=entity_rows) + + assert "driver_id" in str(excinfo.value) + + +@pytest.mark.parametrize("entity_rows", [[], {}], ids=["empty_list", "no_columns"]) +def test_request_without_join_keys_raises_a_client_error_async(entity_rows): + """``entity_rows=[]`` indexed ``[0]`` on the async path too.""" + runner = CliRunner() + with runner.local_repo( + get_example_repo("example_feature_repo_1.py"), "file" + ) as store: + with pytest.raises(MissingJoinKeyValuesException) as excinfo: + asyncio.run( + store.get_online_features_async( + features=FEATURES, entity_rows=entity_rows + ) + ) + + assert "driver_id" in str(excinfo.value)