-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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
Don't reset non-dynamic settings unless explicitly requested #21646
Conversation
AbstractScopedSettings has the ability to only apply updates/deletes to dynamic settings. The flag is currently not respected when a setting is reset/deleted which causes static node settings to be reset if a non-dynamic key is reset via `null` value. Closes elastic#21593
boolean changed = false; | ||
for (String entry : deletes) { | ||
Set<String> keysToRemove = new HashSet<>(); | ||
Set<String> keySet = builder.internalMap().keySet(); | ||
for (String key : keySet) { | ||
if (Regex.simpleMatch(entry, key)) { | ||
if (Regex.simpleMatch(entry, key) && canRemove.test(key)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
given that we already check if we can remove before adding to the deletes set, do we still have to check it here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a comment in the code why we do this here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
++
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
AbstractScopedSettings has the ability to only apply updates/deletes to dynamic settings. The flag is currently not respected when a setting is reset/deleted which causes static node settings to be reset if a non-dynamic key is reset via `null` value. Closes #21593
AbstractScopedSettings has the ability to only apply updates/deletes to dynamic settings. The flag is currently not respected when a setting is reset/deleted which causes static node settings to be reset if a non-dynamic key is reset via `null` value. Closes #21593
AbstractScopedSettings has the ability to only apply updates/deletes
to dynamic settings. The flag is currently not respected when a setting
is reset/deleted which causes static node settings to be reset if a non-dynamic
key is reset via
null
value.Closes #21593