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

Ec2 Discovery: Cleanup deprecated settings #24150

Merged
merged 1 commit into from Apr 19, 2017

Conversation

Projects
None yet
3 participants
@rjernst
Copy link
Member

commented Apr 18, 2017

This commit removes the deprecated cloud.aws.* settings. It also removes
backcompat for specifying discovery.type: ec2, and unused aws signer
code which was removed in a previous PR.

Ec2 Discovery: Cleanup deprecated settings
This commit removes the deprecated cloud.aws.* settings. It also removes
backcompat for specifying `discovery.type: ec2`, and unused aws signer
code which was removed in a previous PR.
@dadoonet
Copy link
Member

left a comment

I just looked at the code for now and it looks good. That's so nice to see so many legacy code being removed!

If you want me to test it for real that will probably take me some time before I can confirm everything is fine although I know we have some internal tools to test that but I think they haven't been updated yet to use secured keys.

LMK if you need to have manual tests before merging it.

@@ -47,6 +47,8 @@ See {plugins}/repository-azure-usage.html#repository-azure-repository-settings[A
* The region setting has been removed. This includes the settings `cloud.aws.region`
and `cloud.aws.ec2.region`. Instead, specify the full endpoint.

* All `cloud.aws.*` and `cloud.aws.ec2.*` settings have been removed. Use `discovery.ec2.*` settings instead.

This comment has been minimized.

Copy link
@dadoonet

dadoonet Apr 19, 2017

Member

Just a thought (nothing important TBH), may be replace with:

All cloud.aws.* and cloud.aws.ec2.* settings have been renamed to discovery.ec2.*.

This comment has been minimized.

Copy link
@rjernst

rjernst Apr 19, 2017

Author Member

I think what is there is better, since the settings were not renamed, but already exist in 5.x.

@rjernst

This comment has been minimized.

Copy link
Member Author

commented Apr 19, 2017

Thanks @dadoonet, i don't think we need manual tests, since this is just removing settings. This should allow us to have much better settings tests since there are not so many variants (I will work on improving them in later followups).

@rjernst rjernst merged commit 151a65e into elastic:master Apr 19, 2017

2 checks passed

CLA Commit author is a member of Elasticsearch
Details
elasticsearch-ci Build finished.
Details

@rjernst rjernst deleted the rjernst:cleanup4 branch Apr 19, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.