Raise ObjectNotFound exception on 404 with JSON response#66
Open
pief wants to merge 1 commit intodavid-caro:masterfrom
pief:404handling
Open
Raise ObjectNotFound exception on 404 with JSON response#66pief wants to merge 1 commit intodavid-caro:masterfrom pief:404handling
pief wants to merge 1 commit intodavid-caro:masterfrom
pief:404handling
Conversation
Contributor
Author
|
Note that this will break existing applications that relied on the behavior of getting an empty list in return. With this implementation, we'll almost always raise ObjectNotFound because Foreman will always return an error in its JSON. At least for GET requests this seems to be too high of a price to me. I'd thus still propose the alternative of not raising the exception for them as a 404 upon a GET is not really an exceptional state. Nothing "failed" here, it is perfectly valid to check if a resource exists at all. |
We used to always return an empty list if we got a HTTP error 404 in response to our requests, ignoring any error message possibly returned by Foreman. For GET requests this is desirable because trying to GET a non-existant resource (eg. using a ID specified by the user) is perfectly normal and does not justify raising an exception. For other requests (especially POST) it is and thus we now raise the ObjectNotFound exception (which so far had not been used anywhere) for anything but GET requests when a 404 is encountered.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We used to always return an empty list if we got a HTTP error 404 in response
to our requests, ignoring any error message possibly returned by Foreman.
This makes us trigger the ObjectNotFound exception (which so far had not been
used anywhere) whenever we find that the 404 is accompanied by a JSON response.