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.

Sign up for GitHub

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

Jump to bottom

Expand allowed status codes #171

Closed
wants to merge 1 commit into from
Closed

Expand allowed status codes #171

wants to merge 1 commit into from

Conversation

pmengelbert
Copy link
Contributor

@pmengelbert pmengelbert commented Jul 10, 2020 •
edited
Loading

POST /v2/<name>/blobs/uploads/?digest=<digest> should allow for 202 Accepted.
All DELETE calls should allow 405 Method Not Allowed.

This PR resolves the following comment by @jdolitsky on issue 68. Resolving this issue will make several registries go green on the relevant workflows.

@pmengelbert
Expand allowed status codes
33a44f4 
POST /v2/<name>/blobs/uploads/?digest=<digest> should allow for 202 Accepted.
All DELETE calls should allow 405 Method Not Allowed.

Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
@jdolitsky
Copy link
Member

jdolitsky commented Jul 10, 2020 •
edited by caniszczyk
Loading

LGTM

Approved with PullApprove

@jdolitsky
Copy link
Member

Should be noted that the current spec is not as permissive, but it will be factored into the rewrite

@jdolitsky jdolitsky mentioned this pull request Jul 10, 2020
Closed
3 tasks
This was referenced Jul 22, 2020
Closed
Closed
@wy65701436 wy65701436 mentioned this pull request Jul 31, 2020
Closed
@vbatts
Copy link
Member

vbatts commented Aug 18, 2020

glad we're testing for this.
I recall the discussion for 202 being used in addition to the 201 written in the spec.
Is there other background on using 202 Accepted that should be linked here? or update to the spec needed to relate to this conformance test?

@jdolitsky
Copy link
Member

@vbatts its something that should be accounted for in the spec reorg: #178

@pmengelbert
Copy link
Contributor Author

@vbatts This has been included in the latest commit for PR #178

@vbatts
Copy link
Member

vbatts commented Sep 16, 2020

the maintainers had chatted about this in a call recently. While everyone noted that some of these response codes are being used just to mimic docker registry, they also noted that they really ought to be corrected in the original registry implementation.

@vbatts
Copy link
Member

vbatts commented Sep 16, 2020

I'm not opposed to this conformance test, but might we track the conversation to have this aligned in the community, rather than only widening the conformance to make it green for more registries?

@jdolitsky
Copy link
Member

FWIW this change is now reflected in the spec.md on master branch

@jdolitsky jdolitsky mentioned this pull request Oct 14, 2020
Open
@pmengelbert pmengelbert mentioned this pull request Oct 29, 2020
Merged
@vbatts
Copy link
Member

vbatts commented Jan 28, 2021

@jdolitsky are saying that this PR is no longer needed? or just that the spec has the language supporting this?

@jdolitsky
Copy link
Member

@vbatts - the spec already contains language, so this PR should prob be merged

Base automatically changed from master to main March 9, 2021 17:38
@jdolitsky
Copy link
Member

closing this. we settled on final HTTP codes for 1.0

@jdolitsky jdolitsky closed this Jun 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@renmaosheng renmaosheng renmaosheng approved these changes

@jdolitsky jdolitsky jdolitsky approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pmengelbert @jdolitsky @vbatts @renmaosheng