Enable deprecation checks for removed settings #53317
Conversation
Today we do not have any infrastructure for adding a deprecation check for settings that are removed. This commit enables this by adding such infrastructure. Note that this infrastructure is unused in this commit, which is deliberate. However, the primary target for this commit is 7.x where this infrastructue will be used, in a follow-up.
|
@elasticmachine update branch |
.../plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/DeprecationChecks.java
Outdated
Show resolved
Hide resolved
…k/deprecation/DeprecationChecks.java
| String.format(Locale.ROOT, "setting [%s] is deprecated and will be removed in the next major version", removedSettingKey); | ||
| final String details = | ||
| String.format(Locale.ROOT, "the setting [%s] is currently set to [%s], remove this setting", removedSettingKey, value); | ||
| return new DeprecationIssue(DeprecationIssue.Level.CRITICAL, message, url, details); |
There was a problem hiding this comment.
should this be WARNING instead of CRITICAL ?
If the setting is removed, won't it be be "archived" allowing the server to continue to function ? I think we are trying to reserve CRITICAL for things items that will prevent the server from running.
There was a problem hiding this comment.
Not necessarily, because these could be node level settings, which we don't archive, which do prevent startup if they are unrecognized. Also, we have discussing removing the archiving of settings, so this behavior could change for cluster-level settings too.
There was a problem hiding this comment.
In that case, should we flex WARNING or CRITICAL based if the setting has node scope ?
Also, should we check that that value is not the default value here so we don't issue an issue a deprecation issue for settings that user never changed ?
There was a problem hiding this comment.
There is a check that the setting is not set on line 18?
There was a problem hiding this comment.
ah .. i missed "Note that fallback settings are excluded." in the exists method.
There was a problem hiding this comment.
Yeah, that's why we have Setting#existsOrFallbackExists.
Today we do not have any infrastructure for adding a deprecation check for settings that are removed. This commit enables this by adding such infrastructure. Note that this infrastructure is unused in this commit, which is deliberate. However, the primary target for this commit is 7.x where this infrastructue will be used, in a follow-up.
Today we do not have any infrastructure for adding a deprecation check for settings that are removed. This commit enables this by adding such infrastructure. Note that this infrastructure is unused in this commit, which is deliberate. However, the primary target for this commit is 7.x where this infrastructure will be used, in a follow-up.