remove dependencies on resumable #2707
Conversation
|
Please sign your commits following these rules: $ git clone -b "go-1.11" git@github.com:davidswu/distribution.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842354293216
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -fAmending updates the existing PR. You DO NOT need to open a new one. |
Signed-off-by: David Wu <david.wu@docker.com>
Signed-off-by: David Wu <david.wu@docker.com>
davidswu
changed the title
Signed-off-by: David Wu <david.wu@docker.com>
Signed-off-by: David Wu <david.wu@docker.com>
Codecov Report
@@ Coverage Diff @@
## master #2707 +/- ##
=========================================
+ Coverage 60.18% 60.4% +0.21%
=========================================
Files 103 103
Lines 8015 8003 -12
=========================================
+ Hits 4824 4834 +10
+ Misses 2548 2527 -21
+ Partials 643 642 -1
Continue to review full report at Codecov.
|
Signed-off-by: David Wu <david.wu@docker.com>
|
This is great, glad we can use the upstream hash libraries again! Can you point to the 1.11 change related to |
|
@dmcgowan |
| } else { | ||
| storedState, err := bw.driver.GetContent(ctx, hashStateMatch.path) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if err = h.Restore(storedState); err != nil { | ||
| // This type assertion is safe since we already did an assertion at the beginning | ||
| if err = h.(encoding.BinaryUnmarshaler).UnmarshalBinary(storedState); err != nil { |
There was a problem hiding this comment.
Is the type assertion necessary? h is already asserted to be a encoded.BinaryMarsheler, right?
There was a problem hiding this comment.
This is a encoding.BinaryUnmarshaler
| return err | ||
| } | ||
|
|
||
| state, err = h.(encoding.BinaryMarshaler).MarshalBinary() |
| @@ -104,18 +104,6 @@ func GetRequestID(ctx context.Context) string { | |||
| // WithResponseWriter returns a new context and response writer that makes | |||
| // interesting response statistics available within the context. | |||
| func WithResponseWriter(ctx context.Context, w http.ResponseWriter) (context.Context, http.ResponseWriter) { | |||
| if closeNotifier, ok := w.(http.CloseNotifier); ok { | |||
There was a problem hiding this comment.
Can you provide a little more context in the commit message as to why the notifier is being removed?
There was a problem hiding this comment.
staticchecks was failing since http.CloseNotifier is deprecated. So we either remove the notifier and rely on Request.Context instead or remove the checks. I opted for the former
| offset := bw.fileWriter.Size() | ||
| if offset == h.Len() { | ||
| if offset == int64(len(state)) { |
There was a problem hiding this comment.
I don't think this is right. In a resumable hash then Len() is the number of bytes that have been written to the hash so far. I believe this is just giving you the len of the digest itself.
There was a problem hiding this comment.
No this is the length of the state not the length of the digest.
There was a problem hiding this comment.
This is wrong though since the state length is not the offset we are looking for. We may need to implement a digester to track the offset written to the hash
| return err | ||
| } | ||
| } | ||
|
|
||
| // Mind the gap. | ||
| if gapLen := offset - h.Len(); gapLen > 0 { | ||
| if gapLen := offset - int64(len(state)); gapLen > 0 { |
Signed-off-by: David Wu <david.wu@docker.com>
closes #2697