Update master branch with latest changes #74
Conversation
| // TODO Remove in 3.0.0 | ||
| settingsFilter.addFilter(KEYSTORE_DEPRECATED); | ||
| settingsFilter.addFilter(PASSWORD_DEPRECATED); | ||
| settingsFilter.addFilter(SUBSCRIPTION_ID_DEPRECATED); |
There was a problem hiding this comment.
Does this mean that if somebody temporary disables the service all settings will become exposed?
|
I think it might be prudent to move settings setup in some component that is always instantiated. Just as precaution. Otherwise the settings changes look good to me. |
|
Closed by mistake |
dadoonet
force-pushed
the
pr/rm-component-settings
branch
from
db888a5
to
4523a37
Compare
|
@imotov I updated the PR. I don't know if it's the best way to achieve it. Let me know what you think. |
| import static org.elasticsearch.cloud.azure.management.AzureComputeService.Management.*; | ||
|
|
||
| public class AzureComputeSettingsFilter extends AbstractLifecycleComponent<AzureSettingsFilter> | ||
| implements AzureSettingsFilter { |
There was a problem hiding this comment.
I think the AzureSettingsFilter interface is not needed - you can just bind concrete implementations. There is also no need for this to be a lifecycle component since lifecycle events are not used anyway.
|
Sorry for the delay. Left one comment. Otherwise LGTM. |
…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
force-pushed
the
pr/rm-component-settings
branch
from
9048e64
to
cd7b8d4
Compare
@imotov We got some changes in elasticsearch master with componentSettings removal.
I refactored the full changes with 2 new commits and would love if you could check them.
Going to send the same PR for 1.x and 1.4 branches.
Thanks!