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
Fix gcs storage driver #3929
Fix gcs storage driver #3929
Conversation
Codecov ReportPatch coverage has no change and project coverage change:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## main #3929 +/- ##
==========================================
+ Coverage 56.88% 56.89% +0.01%
==========================================
Files 106 106
Lines 10703 10684 -19
==========================================
- Hits 6088 6079 -9
+ Misses 3942 3933 -9
+ Partials 673 672 -1 ☔ View full report in Codecov by Sentry. |
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.
I'm LGTM-ing this as it's a port of #3702
I dont want this to be blocked for another year by unproductive conversations - we should look for perfection by iterating before v3 is released.
Who can help with the 2nd approval @milosgajdos ? |
CC: @distribution/maintainers |
both oss and gcs driver were missing the context parameter that is required to satisfy the storagedriver.FileWriter interface. Signed-off-by: Flavian Missi <fmissi@redhat.com>
Signed-off-by: Flavian Missi <fmissi@redhat.com>
Signed-off-by: Flavian Missi <fmissi@redhat.com>
Signed-off-by: Flavian Missi <fmissi@redhat.com>
8ffa64a
to
817dd28
Compare
bumping this as it's still pending a 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.
Based on the discussion in #3702 and the approvals there as well as the minor additional commits added in this to add context to Cancel
and bump some gcs dependencies; I feel this is safe to approve.
great to see this getting merged! thanks to all for driving this effort 👏🏼 |
Big thanks go to @jmontleon for the initial effort and @flavianmissi for getting it across the finish line 🙇 |
Thank you @flavianmissi and @milosgajdos ! |
Based on #3702.
Also fixes the OSS
Cancel
method which was missing a context parameter.