Stop emitting FATAL log when checking existence of indices #982
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Passing `ignore: 404` to Elasticsearch::Client can stop FATAL log in addition to stopping exception. This option is supported since the 5.0.4 and 6.0.0 of `elasticsearch-ruby`. https://github.com/elastic/elasticsearch-ruby/blob/36d45f0c12f4e9ef96646f269748ad33b43e974c/CHANGELOG.md#600 https://github.com/elastic/elasticsearch-ruby/blob/36d45f0c12f4e9ef96646f269748ad33b43e974c/CHANGELOG.md#504 This swallowing was implemented following exception catching as follows. elastic/elasticsearch-ruby@9848c97
Since we are using `ignore: 404` instead of manual `rescue` at `Elasticsearch::Model::Indexing`, there is no need to stub exceptions in the test.
|
Also, this solves the false negatives on index existence which is triggered by non-404 exceptions. |
|
Thank you very much for this contribution @aeroastro! I am adding an issue in |
|
Thank you ❤️ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes #980.
Summary
Passing
ignore: 404to Elasticsearch::Client can stopFATAL log in addition to stopping exception.
This option is supported since the 5.0.4 and 6.0.0 of
elasticsearch-ruby.https://github.com/elastic/elasticsearch-ruby/blob/36d45f0c12f4e9ef96646f269748ad33b43e974c/CHANGELOG.md#600
https://github.com/elastic/elasticsearch-ruby/blob/36d45f0c12f4e9ef96646f269748ad33b43e974c/CHANGELOG.md#504
This swallowing was implemented following exception catching as follows.
elastic/elasticsearch-ruby@9848c97
Test
Unnecessary tests on exceptions are also removed, and following code
covers the required tests.
https://github.com/aeroastro/elasticsearch-rails/blob/c4daaad152517114aa568e326b0388ab8b883e1d/elasticsearch-model/spec/elasticsearch/model/indexing_spec.rb#L652-L661
I have confirmed that the reproduction code on #980 does not emit exceptions nor FATAL log with this patch.
Although the test on not logging
FATAL [404]is omitted in this Pull Request,I believe it is not the responsibility of
Elasticsearch::Model, but that ofElasticsearch::Client.Version Dependency
This code on master branch does not work well with
2.x and 0.1 of
elasticsearchgems, whereignore: 404option is not supported.However, I assume the current maintenance policy does not support old versions
on master branch, and therefore the current code is O.K.
c.f. #981