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

Fixing http status for PUT/PATCH APIs #3444

Merged
merged 2 commits into from Jun 30, 2021

Conversation

sudo-bmitch
Copy link
Contributor

The PUT and PATCH requests should return 201 and 202 respectively.

Signed-off-by: Brandon Mitchell git@bmitch.net

Signed-off-by: Brandon Mitchell <git@bmitch.net>
@codecov-commenter
Copy link

codecov-commenter commented Jun 28, 2021

Codecov Report

Merging #3444 (9c7967a) into main (ad8f5ca) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #3444   +/-   ##
=======================================
  Coverage   56.38%   56.38%           
=======================================
  Files         102      102           
  Lines        7324     7324           
=======================================
  Hits         4130     4130           
  Misses       2541     2541           
  Partials      653      653           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ad8f5ca...9c7967a. Read the comment docs.

@milosgajdos
Copy link
Member

Attaching the URL to the spec for brevity: https://github.com/opencontainers/distribution-spec/blob/v1.0.0/spec.md

Copy link
Collaborator

@wy65701436 wy65701436 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

docs/spec/api.md Outdated Show resolved Hide resolved
docs/spec/api.md Show resolved Hide resolved
@milosgajdos
Copy link
Member

Thanks for stepping in and catching this one @joaodrp 😅 We really need to automate this somehow. @sudo-bmitch can you please address the code comments.

@sudo-bmitch
Copy link
Contributor Author

Signed-off-by: Brandon Mitchell <git@bmitch.net>
@sudo-bmitch
Copy link
Contributor Author

The latest update leaves paginationParameters untouched and the tag/_catalog APIs that use it. Cleaning that up makes better sense for a separate PR.

@joaodrp
Copy link
Collaborator

joaodrp commented Jun 29, 2021

@sudo-bmitch, thanks for the update. I agree that we should leave the pagination stuff for a separate PR. I raised an issue for it: #3445.

@joaodrp
Copy link
Collaborator

joaodrp commented Jun 29, 2021

The checks that are failing here are also failing on main. I raised an issue to fix them (#3446).

I'd like to merge this now but can't as the failing conformance tests are required to pass. This is being discussed in #3442 (comment).

cc @sudo-bmitch

@joaodrp joaodrp merged commit 26b0a79 into distribution:main Jun 30, 2021
dylanrhysscott pushed a commit to digitalocean/docker-distribution that referenced this pull request Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants