Adding a deprecation info API check for fractional byte value settings#77074
Conversation
|
Pinging @elastic/es-data-management (Team:Data Management) |
jbaiera
left a comment
There was a problem hiding this comment.
LGTM
Left some comments, but not sure if any of them are particularly actionable. I wish there were a good/easy way to specifically target the settings that expect byte values, but this should do well enough.
| Map<String, String> fractionalByteSettings = new HashMap<>(); | ||
| for (String key : settings.keySet()) { | ||
| try { | ||
| settings.getAsBytesSize(key, ByteSizeValue.ZERO); |
There was a problem hiding this comment.
This looks like it triggers deprecation logging as well for each fractional value read, do we want that?
| if (stringValue.contains(".")) { | ||
| fractionalByteSettings.put(key, stringValue); | ||
| } | ||
| } catch (Exception ignoreThis) { |
There was a problem hiding this comment.
Hmm, I'm not a huge fan of Exception based control flow. I know that settings are registered with ClusterSettings and IndexScopedSettings so that settings can be validated against the master set, but I don't know if we can filter on the setting's data type due to type erasure, and I'm not sure if those are comprehensive for every setting either.
|
@elasticmachine update branch |
In 6.2.0 we deprecated support for fractional byte value settings. Support was originally going to be
removed altogether in 8.0.0 (see #53927) but that PR was not merged. This commit adds a warning
deprecation info check if any fractional byte values settings are found.
Relates #42404