httpreader: random access over an HTTP resource - #1953
Conversation
|
Conflicts with #1948 |
9c3ce05 to
719faf9
Compare
7635a0a to
01c8352
Compare
BradLugo
left a comment
BradLugo
left a comment
There was a problem hiding this comment.
I think there's enough to address for now. I'll do another deep dive in the next review
28024a7 to
29a4bdc
Compare
06bd4c0 to
1196816
Compare
| if err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
Should we fall back to spooling to disk instead?
There was a problem hiding this comment.
I think any failure of the inspect routine means the spooling would fail, also.
There was a problem hiding this comment.
I'm not so sure, e.g., I can see some weirdness happening where a server/proxy/WAF returns a non-200 when it receives a request with the Range header or something. In any case, after thinking about it for a bit, should we move the inspect after we check the cache anyway?
There was a problem hiding this comment.
Reorganized it so the inspect and fetchFileForCache calls both happen only on the "cache miss" path.
There was a problem hiding this comment.
I still think there may be edge cases where the inspect would fail, but the GET from the spooling would succeed (e.g., a weirdly configured proxy/WAF that blocks Range but would succeed on a regular GET). I think falling back to spooling would be ideal, but I won't block on it. Feel free to resolve this thread and merge.
1196816 to
ea62b17
Compare
2fdeec5 to
ba37694
Compare
ba37694 to
2e72720
Compare
2e72720 to
777fa84
Compare
BradLugo
left a comment
BradLugo
left a comment
There was a problem hiding this comment.
Non-blocking recommendation: #1953 (comment)
This package does an io.ReaderAt over an HTTP resource. It includes a novel way to determine the end of a resource for partially-compliant servers. Using wholly-compliant servers is recommended. Signed-off-by: Hank Donnay <hdonnay@redhat.com> Change-Id: I08d6671217535ac897a2ed5c9cd20aa06a6a6964
Signed-off-by: Hank Donnay <hdonnay@redhat.com> Change-Id: I162632409abf01efcec4633713c9f0d06a6a6964
Signed-off-by: Hank Donnay <hdonnay@redhat.com> Change-Id: I8bd87cb66de93dad080c1d6d8aac34a96a6a6964
Signed-off-by: Hank Donnay <hdonnay@redhat.com> Change-Id: I6c2d6cfae9f6e742056c157e38b8b6156a6a6964
This handles the "easy" case of simply proxying reads for uncompressed tar archives to range requests. Future improvements would move the "spooling" out of this package and into the `fs.FS` implementation. Signed-off-by: Hank Donnay <hdonnay@redhat.com> Change-Id: I9d200dd841954054df0b187b9fd160a56a6a6964
777fa84 to
1b54724
Compare
|
/fast-forward |
2 participants
This adds a package (
internal/httpreader) that implementsio.ReaderAtover HTTP requests. It incorporates a lot of tricks I know from a past life dealing with RFC7233 non- and selectively- compliant servers.Then, the
fetcherpackage gains the capability to use anhttpreader.Readerwhen it notices that a layer is uncompressed.Future work may involve the
tarfslayer doing transparent caching and being able to handle compressed layers directly.