Skip to content

Remove component settings from AbstractComponent #9919

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

Merged
merged 1 commit into from
Feb 27, 2015

Conversation

s1monw
Copy link
Contributor

@s1monw s1monw commented Feb 27, 2015

Today we have two ways of getting a setting, either with the full settings key or with only
the last part of the key where the prefix is implicit depending on the package the class is in via
component settings. this is trappy as well as confusing for users and can break easily if a class is moved
to a new package since the prefix then implicitly changes.
This commit removes the component settings from the codebase.

Today we have two ways of getting a setting, either with the full settings key or with only
the last part of the key where the prefix is implicit depending on the package the class is in via
component settings. this is trappy as well as confusing for users and can break easily if a class is moved
to a new package since the prefix then implicitly changes.
This commit removes the component settings from the codebase.
@dadoonet
Copy link
Member

Note that this will have of course consequences in plugins.

Now that we use full path for setting names, could we define static final String for those?
So we can reuse them in tests and plugins?

@s1monw
Copy link
Contributor Author

s1monw commented Feb 27, 2015

@dadoonet there are no consequences in plugins you always had to specify the full key in the settings.

@mikemccand
Copy link
Contributor

+1 to have just one way to get settings.

@martijnvg
Copy link
Member

+1 the bwc issues that can happen from moving a class makes component settings trappy.

@dadoonet
Copy link
Member

Forgot to say that I'm +1 as well.
About consequences I meant that we need only to fix the code and use fullpath there as well.

@s1monw s1monw merged commit 2b8827d into elastic:master Feb 27, 2015
dadoonet added a commit to elastic/elasticsearch-transport-memcached that referenced this pull request Feb 27, 2015
dadoonet added a commit to elastic/elasticsearch-transport-thrift that referenced this pull request Feb 27, 2015
dadoonet added a commit to elastic/elasticsearch-cloud-aws that referenced this pull request Feb 27, 2015
dadoonet added a commit to dadoonet/elasticsearch-cloud-azure that referenced this pull request Feb 27, 2015
dadoonet added a commit to dadoonet/elasticsearch-cloud-azure that referenced this pull request Feb 27, 2015
dadoonet added a commit to elastic/elasticsearch-lang-javascript that referenced this pull request Feb 28, 2015
dadoonet added a commit to dadoonet/elasticsearch-cloud-azure that referenced this pull request Mar 28, 2015
dadoonet added a commit to dadoonet/elasticsearch-cloud-azure that referenced this pull request Apr 16, 2015
…ponent

Update settings filter to match elastic/elasticsearch#9748
Remove component settings from AbstractComponent as seen in elastic/elasticsearch#9919

Closes elastic#71.
Closes elastic#72.
dadoonet added a commit to elastic/elasticsearch-cloud-azure that referenced this pull request Apr 16, 2015
…ponent

Update settings filter to match elastic/elasticsearch#9748
Remove component settings from AbstractComponent as seen in elastic/elasticsearch#9919

Closes #71.
Closes #72.

(cherry picked from commit cd7b8d4)
dadoonet added a commit to elastic/elasticsearch-cloud-azure that referenced this pull request Apr 16, 2015
…ponent

Update settings filter to match elastic/elasticsearch#9748
Remove component settings from AbstractComponent as seen in elastic/elasticsearch#9919

Closes #71.
Closes #72.

(cherry picked from commit cd7b8d4)
dadoonet added a commit to elastic/elasticsearch-cloud-gce that referenced this pull request Apr 24, 2015
dadoonet added a commit to elastic/elasticsearch-cloud-gce that referenced this pull request Apr 24, 2015
dadoonet added a commit to elastic/elasticsearch-cloud-gce that referenced this pull request Apr 24, 2015
@clintongormley clintongormley changed the title [CORE] Remove component settings from AbstractComponent Remove component settings from AbstractComponent Jun 6, 2015
@clintongormley clintongormley added the :Core/Infra/Core Core issues without another label label Jun 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants