-
Notifications
You must be signed in to change notification settings - Fork 403
docker: convert fully body to partial requests #1359
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
docker: convert fully body to partial requests #1359
Conversation
03ab20d
to
c998c7c
Compare
ready for review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Looks good overall, primarily I’m unsure what guarantees the right order of chunks for the 200 implementation to work correctly; at least I can’t find anything explicitly promising or ensuring that is the case.
docker/docker_image_src.go
Outdated
for _, c := range chunks { | ||
if c.Offset != currentOffset { | ||
if c.Offset < currentOffset { | ||
errs <- errors.New("invalid chunk offset specified") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does anything guarantee the chunks are in increasing, and non-overlapping, offset order? If this is a caller’s responsibility, it needs to be very prominently documented throughout the call stack.
add inline documentation to clarify the expectation for the chunks specified to the GetBlobAt() function. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
ghcr.io converts a multirange request to a 200 response when the client request too much data. If the server replies with a 200 status to a partial request then split the body ignoring any additional content that wasn't requested by the caller. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
c998c7c
to
ea19c43
Compare
thanks for the review! Pushed a new version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks again!
ghcr.io converts a multirange request to a 200 response when the
client request too much data.
If the server replies with a 200 status to a partial request then
split the body ignoring any additional content that wasn't requested
by the caller.
Signed-off-by: Giuseppe Scrivano gscrivan@redhat.com