-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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 aws sdk to validate regions #2687
use aws sdk to validate regions #2687
Conversation
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.
LGTM after lint fix
1153d15
to
cab94e7
Compare
LGTM |
LGTM |
registry/storage/driver/s3-aws/s3.go
Outdated
} { | ||
validRegions[region] = struct{}{} | ||
resolver := endpoints.DefaultResolver() | ||
partitions := resolver.(endpoints.EnumPartitions).Partitions() |
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 see there is also a endpoints.DefaultPartitions()
, would this always return the same list?
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.
Yes let's use that instead
Signed-off-by: David Wu <[email protected]>
cab94e7
to
0b0d470
Compare
Codecov Report
@@ Coverage Diff @@
## master #2687 +/- ##
=======================================
Coverage 59.63% 59.63%
=======================================
Files 99 99
Lines 7893 7893
=======================================
Hits 4707 4707
Misses 2568 2568
Partials 618 618
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.
LGTM
Fixes #2619 |
closes #2619
Signed-off-by: David Wu [email protected]