Do not reference values for filtered settings#48066
Do not reference values for filtered settings#48066danhermann merged 5 commits intoelastic:masterfrom
Conversation
|
Pinging @elastic/es-core-infra (:Core/Infra/Settings) |
|
Thanks, @rjernst. Any opinion on whether the same logic should be applied to typed settings such as int, float, date? |
|
Seems like it should apply to any settings marked as filtered, regardless of type. |
|
👍 I'll make the same changes for all the types, then. |
|
@elasticmachine update branch |
|
@elasticmachine run elasticsearch-ci/packaging-sample |
|
@rjernst, my second commit on this PR removes any mention of values from validation errors on all typed settings with the |
rjernst
left a comment
There was a problem hiding this comment.
Thanks, looks good. Just a comment on some tests.
| Settings build = Settings.builder().put("foo.bar", "I am not a boolean").build(); | ||
| try { | ||
| settingUpdater.apply(build, Settings.EMPTY); | ||
| fail("not a boolean"); |
There was a problem hiding this comment.
Please use expectThrows instead of try/fail/catch
| try { | ||
| predicateSettingUpdater.apply(Settings.builder().put("foo.bar.1.value", "1").put("foo.bar.2.value", "2").build(), | ||
| Settings.EMPTY); | ||
| fail("not accepted"); |
|
@elasticmachine update branch |
Validation exceptions are often logged so the value of filtered properties (of any data type) shouldn't be included.