-
Notifications
You must be signed in to change notification settings - Fork 527
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
Use Data field when fetching in remote #1076
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1076 +/- ##
==========================================
- Coverage 75.49% 75.44% -0.06%
==========================================
Files 107 107
Lines 5129 5159 +30
==========================================
+ Hits 3872 3892 +20
- Misses 709 714 +5
- Partials 548 553 +5
Continue to review full report at Codecov.
|
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.
The addition of a possibly-gigantic data
elements in manifest contents makes me want a remote.WithMaxManifestSize
option (and maybe some sane large default) to prevent resource exhaustion when fetching manifests.
Clients can throw a io.LimitReader
around layer.Compressed()
, but they can't currently easily prevent a huge manifest from being fetched and read.
I think we'd need to rewrite a ton of code if we want to start going down this path. I'd propose we punt on that for now and rethink how this would work in a v2 world. |
Small optimization to use the Data field, if present.
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.
Love it, ship it.
No description provided.