Skip to content
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

blob/s3blob: adds support for per-request s3v2 Options, implementing #3214 #3238

Merged
merged 2 commits into from Apr 4, 2023

Conversation

dkolbly
Copy link
Contributor

@dkolbly dkolbly commented Mar 6, 2023

This PR adds support in the blob/s3blob driver for per-request s3 options when using the underlying v2 AWS SDK.

Specifically, adds As support for:

  • ListOptions.BeforeList (V2) for the type *[]func(*s3v2.Options)
  • ReaderOptions.BeforeRead (V2) for the type *[]func(*s3v2.Options)

which gives the application the ability to supply per-request options when using the V2 SDK.

There does not appear to be any utility in doing the same for BeforeWrite because we already expose access to the s3v2manager.Uploader which includes the ClientOptions list.

Copy link
Contributor

@vangent vangent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly looks great! Couple of small suggestions. Sorry for the delay.

blob/s3blob/s3blob.go Outdated Show resolved Hide resolved
blob/s3blob/s3blob_test.go Outdated Show resolved Hide resolved
This commit addresses PR feedback by fixing the original incorrect
reference to GCS (instead of AWS), and also the two new mentions
introduced in the prior commit.  Also addresses the code style for
the new As test cases to be more consistent with some of the other cases.
@dkolbly
Copy link
Contributor Author

dkolbly commented Apr 4, 2023

I believe I have addressed the feedback, thanks 👍

@codecov
Copy link

codecov bot commented Apr 4, 2023

Codecov Report

Merging #3238 (c62e489) into master (48680fe) will increase coverage by 0.07%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3238      +/-   ##
==========================================
+ Coverage   77.34%   77.42%   +0.07%     
==========================================
  Files         103      103              
  Lines       13548    13552       +4     
==========================================
+ Hits        10479    10492      +13     
+ Misses       2340     2334       -6     
+ Partials      729      726       -3     
Impacted Files Coverage Δ
blob/s3blob/s3blob.go 89.76% <100.00%> (+0.33%) ⬆️

... and 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@vangent vangent merged commit 04a775a into google:master Apr 4, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants