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.

Sign up for GitHub

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

Jump to bottom

Remove either endpoint or region setting in aws cloud plugins #22758

Closed
rjernst opened this issue Jan 24, 2017 · 3 comments · Fixed by #22853 or #23991
Closed

Remove either endpoint or region setting in aws cloud plugins #22758

rjernst opened this issue Jan 24, 2017 · 3 comments · Fixed by #22853 or #23991
Labels
:Distributed Coordination/Discovery-Plugins Anything related to our integration plugins with EC2, GCP and Azure :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs help wanted adoptme

Comments

@rjernst
Copy link
Member

rjernst commented Jan 24, 2017

We have two different settings, endpoint and region, which determine the endpoint that is set on the aws client (and both of these appear unnecessary by default, as neither are required, and the s3 client appears to automatically find the region for a given bucket). My understanding is that it is useful to have an explicit setting to allow connecting to new regions without needing a new release. However, I don't think we need both settings? We should settle on one (I would think endpoint, and remove region altogether?).
@dadoonet
Copy link
Member

I understand the goal of your proposal as it would remove some lines of code.
From a user point of view, I think it's easier to have that kind of feature where someone simply says:

region: eu-west-1

Instead of

endpoint: s3-eu-west-1.amazonaws.com

That said it's not really something you are doing all the time so I guess that we can just link from our documentation to S3 Regions.

Same for discovery-ec2 plugin: we could add a link from our documentation to EC2 regions.

So I don't have a strong opinion about it about removing region setting. For sure, I'd not remove endpoint.

@dadoonet dadoonet mentioned this issue Jan 25, 2017
Closed
@dadoonet
Copy link
Member

Discussed on fix it friday: let's remove the region setting and keep only the endpoint which is much more flexible.

@dadoonet dadoonet added help wanted adoptme and removed discuss labels Jan 27, 2017
rjernst added a commit to rjernst/elasticsearch that referenced this issue Jan 28, 2017
@rjernst
S3 Repository: Deprecate specifying region
959c5ad 
The region and endpoint settings overlap, and have complicated logic for
which to use. Futhermore, region is not necessary, as for most cases it
is automatically figured out by the s3 client, given the bucket name,
and for custom cases like a new region, the endpoint is necessary
anyways. This commit adds a deprecation warning when specifying region.

relates elastic#22758
@rjernst rjernst mentioned this issue Jan 28, 2017
Merged
rjernst added a commit that referenced this issue Jan 29, 2017
@rjernst
S3 Repository: Deprecate specifying region (#22848)
de5ed65 
The region and endpoint settings overlap, and have complicated logic for
which to use. Futhermore, region is not necessary, as for most cases it
is automatically figured out by the s3 client, given the bucket name,
and for custom cases like a new region, the endpoint is necessary
anyways. This commit adds a deprecation warning when specifying region.

relates #22758
rjernst added a commit to rjernst/elasticsearch that referenced this issue Jan 29, 2017
@rjernst
S3 Repository: Remove region setting
125c88d 
This change removes the ability to set region for s3 repositories.
Endpoint should be used instead if a custom s3 location needs to be
used.

closes elastic#22758
@rjernst rjernst mentioned this issue Jan 29, 2017
Merged
rjernst added a commit that referenced this issue Jan 30, 2017
@rjernst
S3 Repository: Remove region setting (#22853)
cf7747c 
This change removes the ability to set region for s3 repositories.
Endpoint should be used instead if a custom s3 location needs to be
used.

closes #22758
@rjernst
Copy link
Member Author

rjernst commented Apr 7, 2017

This still needs to be done for the discovery ec2 plugin.

@rjernst rjernst reopened this Apr 7, 2017
rjernst added a commit to rjernst/elasticsearch that referenced this issue Apr 7, 2017
@rjernst
Discovery EC2: Remove region setting
4b17653 
We have both endpoint and region settings. Region was removed from s3 to
simplify configuration. This is the ec2 equivalent.

closes elastic#22758
@rjernst rjernst mentioned this issue Apr 7, 2017
Merged
rjernst added a commit that referenced this issue Apr 8, 2017
@rjernst
Discovery EC2: Remove region setting (#23991)
83ba677 
We have both endpoint and region settings. Region was removed from s3 to
simplify configuration. This is the ec2 equivalent.

closes #22758
@dadoonet dadoonet mentioned this issue Dec 20, 2017
Closed
@clintongormley clintongormley added :Distributed Coordination/Discovery-Plugins Anything related to our integration plugins with EC2, GCP and Azure :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs and removed :Plugin Discovery EC2 labels Feb 13, 2018
@tlrx tlrx mentioned this issue May 18, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
:Distributed Coordination/Discovery-Plugins Anything related to our integration plugins with EC2, GCP and Azure :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs help wanted adoptme
Projects
None yet
Milestone
No milestone
3 participants
@clintongormley @dadoonet @rjernst