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

adding 404 response for deleteing non-existent resource #1029

Merged
merged 1 commit into from Sep 28, 2016
Merged

adding 404 response for deleteing non-existent resource #1029

merged 1 commit into from Sep 28, 2016

Conversation

@lukasoppermann
Copy link
Contributor

@lukasoppermann lukasoppermann commented Apr 17, 2016

This PR fixes #1023 which is about a not specified response for a DELETE to a resource that does not exist.

@ethanresnick
Copy link
Member

@ethanresnick ethanresnick commented Sep 28, 2016

This looks good to me. Any objections @json-api/owners?

One question is whether we should say a 404 MAY be returned or SHOULD be returned. I'm tempted to say SHOULD, because HTTP's most recent specification pretty clearly implies that DELETE being idempotent doesn't mean you have to return the same status code each time (which was the argument for MAY in #1023):

Idempotent methods are distinguished because the request can be repeated automatically if a communication failure occurs before the client is able to read the server's response... It knows that repeating the request will have the same intended effect, even if the original request succeeded, though the response might differ. (Emphasis added.)

But, if there's disagreement on a SHOULD, I don't think that technical debate should hold this up, as we can always strengthen a MAY to a SHOULD later.

Loading

@lukasoppermann
Copy link
Contributor Author

@lukasoppermann lukasoppermann commented Sep 28, 2016

Even though I am not part of the owners, I would be in favour of SHOULD.

Loading

@dgeb
Copy link
Member

@dgeb dgeb commented Sep 28, 2016

I am fine strengthening to a SHOULD here as well. I think a MAY is almost meaningless compared with the general catchall response section.

Loading

@dgeb
Copy link
Member

@dgeb dgeb commented Sep 28, 2016

@lukasoppermann please update to SHOULD and then we'll get this merged.

Loading

@lukasoppermann
Copy link
Contributor Author

@lukasoppermann lukasoppermann commented Sep 28, 2016

@dgeb updated.

Loading

@dgeb dgeb merged commit 15e4b7a into json-api:gh-pages Sep 28, 2016
@dgeb
Copy link
Member

@dgeb dgeb commented Sep 28, 2016

Thanks @lukasoppermann! :shipit:

Loading

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

Successfully merging this pull request may close these issues.

3 participants