Fix for elasticsearch.index_template_present fails on ES <6.x #48442#48445
Fix for elasticsearch.index_template_present fails on ES <6.x #48442#48445rallytime merged 2 commits intosaltstack:developfrom babs:fix-issue-48442-elasticsearch
Conversation
| if check_definition: | ||
| definition_to_diff = {'aliases': {}, 'mappings': {}, 'settings': {}} | ||
| definition_to_diff.update(definition) | ||
| if type(definition) == str: |
There was a problem hiding this comment.
I think using an isinstance check here is more clear.
| diff = __utils__['dictdiffer.deep_diff'](current_template, definition_to_diff) | ||
| # Prune empty keys (avoid false positive diff) | ||
| for key in ("mappings", "aliases", "settings"): | ||
| if current_template[key] == {}: |
There was a problem hiding this comment.
The check here it not th know if it's a dict but and empty dict.
I find isinstance(current_template[key], dict) and len(current_template[key].key() == 0) a little bit too wordy for that purpose, any suggestion ?
There was a problem hiding this comment.
What about if not current_template[key]:?
There was a problem hiding this comment.
if not current_template[key]: would also match non dict typed values, I'll check if it might have any border effect as if an empty string was an expected value
There was a problem hiding this comment.
You're right and this is fine. It just felt that an awkward check to me.
|
Hi @rallytime, should I squash the commits ? |
|
Have we tested this change on EL 6 and above as well? |
|
@cachedout yes I did, I've both in prod using it (5.x and 6+). |
3 participants
What does this PR do?
It resolv a bug related to index template for Elasticsearch before 6.x
What issues does this PR fix or reference?
It fixes issue #48442.
Tests written?
No
Commits signed with GPG?
No