From a0a7316b9e0b1e4064db798d9b229a9eabbeabb9 Mon Sep 17 00:00:00 2001 From: Rob Howley Date: Wed, 14 Sep 2022 21:02:26 -0400 Subject: [PATCH 1/5] return 422 on bad push source name Signed-off-by: Rob Howley --- sdk/python/feast/errors.py | 5 +++++ sdk/python/feast/feature_server.py | 6 ++++++ sdk/python/feast/feature_store.py | 4 ++++ .../integration/e2e/test_python_feature_server.py | 13 +++++++++++++ 4 files changed, 28 insertions(+) diff --git a/sdk/python/feast/errors.py b/sdk/python/feast/errors.py index 15fda6ac7d0..e9f16213ee5 100644 --- a/sdk/python/feast/errors.py +++ b/sdk/python/feast/errors.py @@ -384,3 +384,8 @@ def __init__(self, features: Any): super().__init__( f"Invalid `features` parameter type {type(features)}. Expected one of List[str] and FeatureService." ) + + +class PushSourceNotFoundException(Exception): + def __init__(self, push_source_name: str): + super().__init__(f"Unable to find push source '{push_source_name}'.") diff --git a/sdk/python/feast/feature_server.py b/sdk/python/feast/feature_server.py index c2596d411c6..7b0cfc4bed8 100644 --- a/sdk/python/feast/feature_server.py +++ b/sdk/python/feast/feature_server.py @@ -13,6 +13,7 @@ import feast from feast import proto_json from feast.data_source import PushMode +from feast.errors import PushSourceNotFoundException from feast.protos.feast.serving.ServingService_pb2 import GetOnlineFeaturesRequest @@ -98,6 +99,11 @@ def push(body=Depends(get_body)): allow_registry_cache=request.allow_registry_cache, to=to, ) + except PushSourceNotFoundException as e: + # Print the original exception on the server side + logger.exception(traceback.format_exc()) + # Raise HTTPException to return the error message to the client + raise HTTPException(status_code=422, detail=str(e)) except Exception as e: # Print the original exception on the server side logger.exception(traceback.format_exc()) diff --git a/sdk/python/feast/feature_store.py b/sdk/python/feast/feature_store.py index 9350220c21c..07ac9dd5015 100644 --- a/sdk/python/feast/feature_store.py +++ b/sdk/python/feast/feature_store.py @@ -59,6 +59,7 @@ EntityNotFoundException, FeatureNameCollisionError, FeatureViewNotFoundException, + PushSourceNotFoundException, RequestDataNotFoundInEntityDfException, RequestDataNotFoundInEntityRowsException, ) @@ -1444,6 +1445,9 @@ def push( ) } + if not fvs_with_push_sources: + raise PushSourceNotFoundException(push_source_name) + for fv in fvs_with_push_sources: if to == PushMode.ONLINE or to == PushMode.ONLINE_AND_OFFLINE: self.write_to_online_store( diff --git a/sdk/python/tests/integration/e2e/test_python_feature_server.py b/sdk/python/tests/integration/e2e/test_python_feature_server.py index 9c61f6fa198..383eef378b4 100644 --- a/sdk/python/tests/integration/e2e/test_python_feature_server.py +++ b/sdk/python/tests/integration/e2e/test_python_feature_server.py @@ -84,6 +84,19 @@ def test_push(python_fs_client): ) == [initial_temp * 100] +@pytest.mark.integration +@pytest.mark.universal_online_stores +def test_push_source_does_not_exist(python_fs_client): + response = python_fs_client.post( + "/push", + data={ + "push_source_name": "push_source_does_not_exist", + "df": {}, + }, + ) + assert response.status_code == 422 + + def _get_temperatures_from_feature_server(client, location_ids: List[int]): get_request_data = { "features": ["pushable_location_stats:temperature"], From c084b1b098a70269cb979b5d59e9b38ac0fb26b2 Mon Sep 17 00:00:00 2001 From: Rob Howley Date: Thu, 15 Sep 2022 10:36:36 -0400 Subject: [PATCH 2/5] fix: cant send empty df Signed-off-by: Rob Howley --- .../integration/e2e/test_python_feature_server.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/sdk/python/tests/integration/e2e/test_python_feature_server.py b/sdk/python/tests/integration/e2e/test_python_feature_server.py index 383eef378b4..5fc2f174935 100644 --- a/sdk/python/tests/integration/e2e/test_python_feature_server.py +++ b/sdk/python/tests/integration/e2e/test_python_feature_server.py @@ -87,11 +87,19 @@ def test_push(python_fs_client): @pytest.mark.integration @pytest.mark.universal_online_stores def test_push_source_does_not_exist(python_fs_client): + initial_temp = _get_temperatures_from_feature_server( + python_fs_client, location_ids=[1] + )[0] response = python_fs_client.post( "/push", data={ "push_source_name": "push_source_does_not_exist", - "df": {}, + "df": { + "location_id": [1], + "temperature": [initial_temp * 100], + "event_timestamp": [str(datetime.utcnow())], + "created": [str(datetime.utcnow())], + }, }, ) assert response.status_code == 422 From 570f674b59b9e1fa464d8d22caee131fe19f0adc Mon Sep 17 00:00:00 2001 From: Rob Howley Date: Wed, 14 Sep 2022 21:02:26 -0400 Subject: [PATCH 3/5] return 422 on bad push source name Signed-off-by: Rob Howley --- sdk/python/feast/errors.py | 5 +++++ sdk/python/feast/feature_server.py | 6 ++++++ sdk/python/feast/feature_store.py | 4 ++++ .../integration/e2e/test_python_feature_server.py | 13 +++++++++++++ 4 files changed, 28 insertions(+) diff --git a/sdk/python/feast/errors.py b/sdk/python/feast/errors.py index 834df0e5c48..15ba86781df 100644 --- a/sdk/python/feast/errors.py +++ b/sdk/python/feast/errors.py @@ -398,3 +398,8 @@ def __init__(self): super().__init__( "The entity dataframe specified does not have the timestamp field as a datetime." ) + + +class PushSourceNotFoundException(Exception): + def __init__(self, push_source_name: str): + super().__init__(f"Unable to find push source '{push_source_name}'.") diff --git a/sdk/python/feast/feature_server.py b/sdk/python/feast/feature_server.py index c2596d411c6..7b0cfc4bed8 100644 --- a/sdk/python/feast/feature_server.py +++ b/sdk/python/feast/feature_server.py @@ -13,6 +13,7 @@ import feast from feast import proto_json from feast.data_source import PushMode +from feast.errors import PushSourceNotFoundException from feast.protos.feast.serving.ServingService_pb2 import GetOnlineFeaturesRequest @@ -98,6 +99,11 @@ def push(body=Depends(get_body)): allow_registry_cache=request.allow_registry_cache, to=to, ) + except PushSourceNotFoundException as e: + # Print the original exception on the server side + logger.exception(traceback.format_exc()) + # Raise HTTPException to return the error message to the client + raise HTTPException(status_code=422, detail=str(e)) except Exception as e: # Print the original exception on the server side logger.exception(traceback.format_exc()) diff --git a/sdk/python/feast/feature_store.py b/sdk/python/feast/feature_store.py index 9350220c21c..07ac9dd5015 100644 --- a/sdk/python/feast/feature_store.py +++ b/sdk/python/feast/feature_store.py @@ -59,6 +59,7 @@ EntityNotFoundException, FeatureNameCollisionError, FeatureViewNotFoundException, + PushSourceNotFoundException, RequestDataNotFoundInEntityDfException, RequestDataNotFoundInEntityRowsException, ) @@ -1444,6 +1445,9 @@ def push( ) } + if not fvs_with_push_sources: + raise PushSourceNotFoundException(push_source_name) + for fv in fvs_with_push_sources: if to == PushMode.ONLINE or to == PushMode.ONLINE_AND_OFFLINE: self.write_to_online_store( diff --git a/sdk/python/tests/integration/e2e/test_python_feature_server.py b/sdk/python/tests/integration/e2e/test_python_feature_server.py index 9c61f6fa198..383eef378b4 100644 --- a/sdk/python/tests/integration/e2e/test_python_feature_server.py +++ b/sdk/python/tests/integration/e2e/test_python_feature_server.py @@ -84,6 +84,19 @@ def test_push(python_fs_client): ) == [initial_temp * 100] +@pytest.mark.integration +@pytest.mark.universal_online_stores +def test_push_source_does_not_exist(python_fs_client): + response = python_fs_client.post( + "/push", + data={ + "push_source_name": "push_source_does_not_exist", + "df": {}, + }, + ) + assert response.status_code == 422 + + def _get_temperatures_from_feature_server(client, location_ids: List[int]): get_request_data = { "features": ["pushable_location_stats:temperature"], From a4ff8c1afa1c97cbd06f08e8e875cb2225573f6f Mon Sep 17 00:00:00 2001 From: Rob Howley Date: Thu, 15 Sep 2022 10:36:36 -0400 Subject: [PATCH 4/5] fix: cant send empty df Signed-off-by: Rob Howley --- .../integration/e2e/test_python_feature_server.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/sdk/python/tests/integration/e2e/test_python_feature_server.py b/sdk/python/tests/integration/e2e/test_python_feature_server.py index 383eef378b4..5fc2f174935 100644 --- a/sdk/python/tests/integration/e2e/test_python_feature_server.py +++ b/sdk/python/tests/integration/e2e/test_python_feature_server.py @@ -87,11 +87,19 @@ def test_push(python_fs_client): @pytest.mark.integration @pytest.mark.universal_online_stores def test_push_source_does_not_exist(python_fs_client): + initial_temp = _get_temperatures_from_feature_server( + python_fs_client, location_ids=[1] + )[0] response = python_fs_client.post( "/push", data={ "push_source_name": "push_source_does_not_exist", - "df": {}, + "df": { + "location_id": [1], + "temperature": [initial_temp * 100], + "event_timestamp": [str(datetime.utcnow())], + "created": [str(datetime.utcnow())], + }, }, ) assert response.status_code == 422 From 2397095bb52d66f2a6e946bae55d3e69e72b5cae Mon Sep 17 00:00:00 2001 From: Rob Howley Date: Tue, 27 Sep 2022 21:29:18 -0400 Subject: [PATCH 5/5] fix: json.dumps the body in the post :facepalm: Signed-off-by: Rob Howley --- .../e2e/test_python_feature_server.py | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/sdk/python/tests/integration/e2e/test_python_feature_server.py b/sdk/python/tests/integration/e2e/test_python_feature_server.py index 5fc2f174935..089efd7a562 100644 --- a/sdk/python/tests/integration/e2e/test_python_feature_server.py +++ b/sdk/python/tests/integration/e2e/test_python_feature_server.py @@ -92,15 +92,17 @@ def test_push_source_does_not_exist(python_fs_client): )[0] response = python_fs_client.post( "/push", - data={ - "push_source_name": "push_source_does_not_exist", - "df": { - "location_id": [1], - "temperature": [initial_temp * 100], - "event_timestamp": [str(datetime.utcnow())], - "created": [str(datetime.utcnow())], - }, - }, + data=json.dumps( + { + "push_source_name": "push_source_does_not_exist", + "df": { + "location_id": [1], + "temperature": [initial_temp * 100], + "event_timestamp": [str(datetime.utcnow())], + "created": [str(datetime.utcnow())], + }, + } + ), ) assert response.status_code == 422