Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for Elasticsearch v7 #1216

Merged
merged 3 commits into from Jun 9, 2022

Conversation

markus1978
Copy link
Contributor

@markus1978 markus1978 commented Jun 1, 2022

Elasticsearch 6.8 lost its official support in February 2022. So we should go for Elasticsearch 7, I guess.

  • use 7.x in the ci
  • use 7.x in the install docs
  • added a fix, because 7.x is capping out the total hits instead of actually trying to compute them by default
  • fix the index creation. Before, we were trying to add an explicit mapping. But the mapping json was wrong. Therefore, no mapping had been applied effectively and we were running in a dynamic mapping. However, this dynamic mapping did not work with ES 7 anymore due to slight changes in ES. I fixed the mapping so that the ES indices have an explicit non dynamic mapping now.

@markus1978 markus1978 force-pushed the ms/elasticsearch-7 branch 2 times, most recently from 0041754 to 0fb97d4 Compare June 1, 2022 11:47
@ml-evs ml-evs added enhancement New feature or request transformer/Elasticsearch Related to filter transformer for Elasticsearch labels Jun 1, 2022
@ml-evs ml-evs changed the title Ms/elasticsearch 7 Support for Elasticsearch v7 Jun 1, 2022
@codecov
Copy link

codecov bot commented Jun 3, 2022

Codecov Report

Merging #1216 (4fd5aca) into master (1f02600) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 4fd5aca differs from pull request most recent head 3ae2850. Consider uploading reports for the commit 3ae2850 to get more accurate results

@@           Coverage Diff           @@
##           master    #1216   +/-   ##
=======================================
  Coverage   91.27%   91.28%           
=======================================
  Files          72       72           
  Lines        4285     4289    +4     
=======================================
+ Hits         3911     3915    +4     
  Misses        374      374           
Flag Coverage Δ
project 91.28% <100.00%> (+<0.01%) ⬆️
validator 90.67% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
optimade/server/entry_collections/elasticsearch.py 97.53% <100.00%> (+0.12%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1f02600...3ae2850. Read the comment docs.

@markus1978
Copy link
Contributor Author

@ml-evs I finally fixed all the ES7 issues. This now works for the OPT tests and for our implementation.

@ml-evs ml-evs added the server Issues pertaining to the example server implementation label Jun 9, 2022
Copy link
Member

@ml-evs ml-evs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @markus1978!

.github/workflows/ci.yml Show resolved Hide resolved
ml-evs
ml-evs previously approved these changes Jun 9, 2022
@ml-evs ml-evs enabled auto-merge (squash) June 9, 2022 18:57
@ml-evs ml-evs disabled auto-merge June 9, 2022 19:57
@ml-evs ml-evs enabled auto-merge (squash) June 9, 2022 19:57
@ml-evs ml-evs merged commit c57a0ac into Materials-Consortia:master Jun 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request server Issues pertaining to the example server implementation transformer/Elasticsearch Related to filter transformer for Elasticsearch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants