-
Notifications
You must be signed in to change notification settings - Fork 995
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
fix: Added Online Store REST client errors handler #4488
fix: Added Online Store REST client errors handler #4488
Conversation
Signed-off-by: Theodor Mihalache <tmihalac@redhat.com>
utils.make_tzaware(parser.parse(request.end_ts)), request.feature_views | ||
) | ||
|
||
@app.exception_handler(Exception) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice, I didn't realize FastAPI had a global exception handler.
https://fastapi.tiangolo.com/tutorial/handling-errors/#install-custom-exception-handlers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm apart the comment on errors
module
- Small refactor to from_error_detail and FeastErrors - Fixed tests Signed-off-by: Theodor Mihalache <tmihalac@redhat.com>
- Fixed linter Signed-off-by: Theodor Mihalache <tmihalac@redhat.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
What this PR does / why we need it:
Added a decorator for rest calls to remote online server failures and adds an additional message to the response with the failure status and details.
The proxy client then rebuilds the original error from the status data.
Which issue(s) this PR fixes:
Fixes #4481