Detect duplicate settings keys on startup #13086
Conversation
jasontedor
added
>bug
review
:Core/Infra/Core
|
I didn't really look close enough here but I wonder what happen if I have |
|
@s1monw I can push a commit that will cause that to fail too. |
|
|
||
| /** | ||
| * | ||
| */ | ||
| public class JsonSettingsLoaderTests extends ESTestCase { | ||
| @Rule | ||
| public ExpectedException expectedException = ExpectedException.none(); |
There was a problem hiding this comment.
No, it has to be specified.
|
@jasontedor should it fail? I mean I would expect the -D to override whatever is configured? |
|
Sorry for the miscommunication here; I misunderstood your first question as you implicitly wanting that change. To be clear, with this pull request as it currently stands, the only change is that duplicates in the configuration file will cause an exception at startup. Settings can still be specified on the command line and will not cause an exception if settings with the same key are also in the configuration file. Settings on the command line still take precedence. |
| @Test | ||
| public void testDuplicateKeysThrowsException() { | ||
| expectedException.expect(SettingsException.class); | ||
| expectedException.expectCause(Matchers.<Throwable>instanceOf(ElasticsearchParseException.class)); |
There was a problem hiding this comment.
Can we just use a simple try/catch here? I don't see why we need to use hamcrest complicatedness...
There was a problem hiding this comment.
or junit for that matter. try/catch is much more readable (and the way most other tests do this)
There was a problem hiding this comment.
ACK. I pushed a commit that I agree is simpler.
|
lgtm |
This commit changes the startup behavior of Elasticsearch to throw an exception if duplicate settings keys are detected in the Elasticsearch configuration file. Closes #13079
Detect duplicate settings keys on startup
|
Thanks for the helpful review @rjernst. |
This commit changes the startup behavior of Elasticsearch to throw an
exception if duplicate settings keys are detected in the Elasticsearch
configuration file.
Closes #13079