Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Push api call to a push source that doesn't exist returns 200 #3210

Closed
robhowley opened this issue Sep 13, 2022 · 1 comment · Fixed by #3214
Closed

Push api call to a push source that doesn't exist returns 200 #3210

robhowley opened this issue Sep 13, 2022 · 1 comment · Fixed by #3214

Comments

@robhowley
Copy link
Contributor

Expected Behavior

When we POST to a push source that doesn't exist like so

curl -X POST "http://localhost:6566/push" -d '{
    "push_source_name": "push_source_name_that_does_not_exist",
    "df": {
            "some_entity": [1001],
            "event_timestamp": ["2022-05-13 10:59:42"],
            "created": ["2022-05-13 10:59:42"],
            ...
    },
    "to": "online_and_offline",
  }' | jq

I'd expect an error. It is a valid format to the correct endpoint, but is an entity that doesn't exist. A 422 unprocessable entity error would probably make sense here.

Current Behavior

Method FeatureStore().push receives a push_source_name indicating which feature views are impacted by the new data being pushed. The set of feature views is filtered down here. If a push source name is provided that doesn't actually exist then the resulting set is empty, the loop is skipped and the function is a no-op. This successful return means that the /push call in feature_server.py never throws to return an error code.

Steps to reproduce

  • start a local feature server
  • make a push call to a push source that does not exist

Specifications

  • Version:
  • Platform:
  • Subsystem:

Possible Solution

  • Raise a custom exception when no push source is found with the provided name
  • Catch that exception in the feature server
  • Return 422 in that case

If this is considered reasonable, I'm happy to make the contribution myself.

@adchia
Copy link
Collaborator

adchia commented Sep 14, 2022

That generally seems fine. Ideally you'd give a detailed error message as well explaining why it failed too, but sgtm!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants