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

Add more regions to registry S3 storage driver #2140

Merged
merged 2 commits into from
Jan 18, 2017

Conversation

mfojtik
Copy link
Contributor

@mfojtik mfojtik commented Jan 11, 2017

Signed-off-by: Michal Fojtik [email protected]

@dmcgowan
Copy link
Collaborator

LGTM

Looks like there are at least 2 others not in the list, ap-south-1 and eu-west-2

@mfojtik
Copy link
Contributor Author

mfojtik commented Jan 11, 2017

@dmcgowan I can add them as well here, thx for noticing.

Namely adding ca-central-1, ap-south-1 and eu-west-1.

Signed-off-by: Michal Fojtik <[email protected]>
@mfojtik mfojtik changed the title Add 'ca-central-1' region for registry S3 storage driver Add more regions to registry S3 storage driver Jan 11, 2017
@pweil-
Copy link

pweil- commented Jan 11, 2017

LGTM, thanks @mfojtik

@dmcgowan
Copy link
Collaborator

LGTM

@dmcgowan dmcgowan added this to the Registry/2.6 milestone Jan 11, 2017
@mfojtik
Copy link
Contributor Author

mfojtik commented Jan 11, 2017

(note that I haven' tested this yet and I'm worried that I can't grep these endpoint names in aws-sdk-go)

@dmcgowan
Copy link
Collaborator

Might need to upgrade the dependency to 1.6.3 then aws/aws-sdk-go@ad048b5

Looks like ap-south-1 and ca-central-1 were added in 1.6.2, eu-west-2 in 1.6.3 aws/aws-sdk-go@1355b45

@dmcgowan
Copy link
Collaborator

Looks like upgrading the dependency will pull in a lot more changes, I am going to move this back to 2.7 release. Maybe could consider for 2.6.1 if it is important for you guys and you have time to test it.

@dmcgowan dmcgowan modified the milestones: Registry/2.7, Registry/2.6 Jan 11, 2017
@codecov-io
Copy link

codecov-io commented Jan 11, 2017

Current coverage is 51.12% (diff: 100%)

Merging #2140 into master will decrease coverage by 10.00%

@@             master      #2140   diff @@
==========================================
  Files           125        125           
  Lines         11432      11435      +3   
  Methods           0          0           
  Messages          0          0           
  Branches          0          0           
==========================================
- Hits           6988       5846   -1142   
- Misses         3556       4843   +1287   
+ Partials        888        746    -142   

Powered by Codecov. Last update 69c7f30...44dfd1b

@mfojtik
Copy link
Contributor Author

mfojtik commented Jan 12, 2017

@dmcgowan i bumped aws dep to 1.6.3 and we will do some testing in OpenShift to make sure it works well. The distribution tests seems to pass.

@miminar @legionus PTAL

Copy link
Contributor

@miminar miminar left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -1,6 +1,6 @@
github.com/Azure/azure-sdk-for-go c6f0533defaaaa26ea4dff3c9774e36033088112
github.com/Sirupsen/logrus d26492970760ca5d33129d2d799e34be5c4782eb
github.com/aws/aws-sdk-go 90dec2183a5f5458ee79cbaf4b8e9ab910bc81a6
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like the new hash is for 1.6.9, that is fine but was this intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dmcgowan sorry, 1.6.9 is what I bumped it to :-) (ETOOMUCHPR's ;-)

@dmcgowan
Copy link
Collaborator

LGTM

@nwt
Copy link
Contributor

nwt commented Jan 18, 2017

LGTM.

@dmcgowan dmcgowan merged commit 954b4e8 into distribution:master Jan 18, 2017
stelund referenced this pull request Jan 27, 2017
Updated to latest version of go aws sdk.
Use vendored sub pakages within aws sdk.
Adds missing vendor packages for letsencrypt

Fixes #1832

Signed-off-by: Derek McGowan <[email protected]> (github: dmcgowan)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants