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

400 and 404 responses should not raise exceptions when they are expected #11

Closed
Quantra opened this issue Aug 12, 2023 · 2 comments
Closed
Assignees

Comments

@Quantra
Copy link

Quantra commented Aug 12, 2023

Issue:

In my application I need to be able to handle when the API returns with a 400 status code for a "create" endpoint so I can display the returned form errors to the user.

Currently when a 400 is returned a RuntimeError is raised with a message along the lines of Unexpected status_code: 400, result: {"non_field_errors": ...} this means the result is swallowed along with the status code.

In this case I don't think any exception should be raised because we can expect a 400 as a valid response for the API.

The same is true for when a 404 is returned for a "delete" or "update" endpoint.

Proposed solution:

Update the appropriate calls to http_post_result throughout and add the appropriate status in the ensure_status kwarg.

I can make a PR for this but it's not as simple as this. There are considerations at least for when wait=True in ApiModelManager.create and I expect there will be more. If I can get a test API key so I can run the tests I'll gladly proceed.

@yqdv
Copy link
Contributor

yqdv commented Feb 2, 2024

@Quantra
First, I apologize for missing these two issues you opened almost six months ago. I just noticed them now while working on a feature request.

I suppose you probably went with a workaround of calling the API directly for this particular event.

Your proposed solution looks like the right approach, and I need to reproduce it in order to inspect the issue in detail. I think what you are describing would be something like a user creating an object with a name that's already in use- we want to return to the user the actual API error. Internally, we use this library in infallible cases and so this valid criticism never came up.

Regarding a test key, we could set something like this up. To be clear, the tests do run using a normal API key for any opalstack account, but since the tests delete data it's generally not something you'd run under the same account where your production code is running, just to be safe. Are you still using the opalstack-python library and is this issue still relevant to your needs?

@yqdv
Copy link
Contributor

yqdv commented Feb 15, 2024

This can be solved as a byproduct of solving #12 , so closing this ticket, while keeping in mind that the solution to #12 should satisfy the needs of this issue.

@yqdv yqdv closed this as completed Feb 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants