Using aws-sdk-s3 #855
Merged
Using aws-sdk-s3 #855
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JackDanger
force-pushed
the
jackdanger/using-aws-sdk-s3
branch
from
3fae903 to
ac069a1
Compare
lamont-granquist
approved these changes
Sep 26, 2018
|
someone should probably flame test this, since I think this is more or less equivalent to an upgrade to aws-sdk-v3(?), and IDK what the rspec testing looks like. |
tas50
approved these changes
Sep 26, 2018
|
@lamont-granquist What does |
|
someone internal to manually test + double check |
|
I figured y'all are busy so I ran some tests on this myself. It seems to have identical functionality to what it had prior to this change: (These happen to be the 3 objects in the given S3 bucket I used as an example) |
|
@lamont-granquist Pinging on this – I manually tested this myself because I assume nobody's going to prioritize this internally (y'all have real work to do but this week this is my real work :D ) |
|
looks busted on chef/chef https://github.com/chef/chef/blob/master/omnibus/Gemfile#L3 changing that line to: gem "omnibus", git: "https://github.com/JackDanger/omnibus", branch: "jackdanger/using-aws-sdk-s3" and then doing a "bundle update && bundle exec omnibus build chef" results in: |
JackDanger
force-pushed
the
jackdanger/using-aws-sdk-s3
branch
from
5b2d338 to
519b214
Compare
|
@lamont-granquist Thanks for sharing a way I can better test this. It looks like the aws-sdk-v3 throws an error when the AWS credentials are blank and no fallback has been specified. I've added 519b214 which fixes this. |
The full aws-sdk gem includes a massive number of dependency gems now that AWS has broken the aws-sdk into many pieces. Trimming this dependency allows applications that depend on omnibus to avoid downloading dozens or hundreds of small packages when they run 'bundle install' Signed-off-by: Jack Danger <[email protected]>
Signed-off-by: Jack Danger <[email protected]>
JackDanger
force-pushed
the
jackdanger/using-aws-sdk-s3
branch
from
519b214 to
185f21d
Compare
|
@lamont-granquist I just rebased this and would love a re-review after fixing the nil credentials problem you found. |
|
yeah that fixed the S3Fetcher. |
lamont-granquist
approved these changes
Oct 11, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The full aws-sdk gem includes a massive number of dependency gems now that AWS has broken the aws-sdk into many pieces. Trimming this dependency allows applications that depend on omnibus to avoid downloading dozens or hundreds of small packages when they run 'bundle install'