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

Add support for Elasticsearch 7.0 and templates #61

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

reachfh
Copy link

@reachfh reachfh commented Nov 17, 2019

Elasticsearch 7.0 removes types from indexes, which has some incompatible changes to the APIs. This PR adds support while keeping compatibility. Mostly this works fine, except that I could not figure out a way to avoid the obsolete types list in Elastix.Search.search.

Add support for managing templates in a new Elastix.Templates module.

Make Elastix.Bulk.post_raw/4 accept builk data in iodata format, allowing encoding errors to be handled before calling the API.

Use Logger instead of IO to print deprecation warnings, making it easier to log messages tests.

Make general code changes:

  • Use the same functions to handle arguments in all modules
  • Use basic types for specs to make them shorter and easier to read
  • Avoid piping when it is clearer
  • Use use access protocol for keyword access instead of Keyword.get
  • Use shorter names for vars to avoid breaking lines
  • Use head matching style instead of if/else

Add more documentation and examples for functions. The home page is still using the old pre-7.x API. At some point the new API should be the default.

Make improvements to tests:

  • Adding Dializer and fixing issues
  • Making lines shorter by using variables and splitting requests from asserts
  • Fixes a problem in snapshot where it would try to delete a snapshot before processing had completed
  • Use the Elasticsearch version to customize tests
  • Added old tests in test/elastix/old with just the minimum changes required to make them pass with the new code

It has mainly been tested with Elasticsearch 6.8 and 7.4.

Elasticsearch 7.0 removes types from indexes, which has some
incompatible changes to the APIs. This PR adds support
wile keeping compatibility. Mostly this works fine,
except that I could not figure out a way to avoid the
obsolete types list in Elastix.Search.search.

Add support for managing templates in a new Elastix.Templates module.

Make Elastix.Bulk.post_raw/4 accept builk data in iodata format,
allowing encoding errors to be handled before calling the API.

Use Logger instead of IO to print deprecation warnings, making it
easier to log messages tests.

Make general code changes:

* Use the same functions to handle arguments in all modules
* Use basic types for specs to make them shorter and easier to read
* Avoid piping when it is clearer
* Use use access protocal for keyword access instead of Keyword.get
* Use shorter names for vars to avoid breaking lines
* Use head matching style instead of if/else

Add more documentation and examples for functions. The home page is
still using the old pre-7.x API. At some point the new API should be the
default.

Make improvements to tests:

* Adding Dializer and fixing issues
* Making lines shorter by using variables and splitting requests from
  asserts
* Fixes a problem in snapshot where it would try to delete a snapshot
  before processing had completed
* Use the Elasticsearch version to customize tests
* Added old tests in test/elastix/old with just the minimum changes
  required to make them pass with the new code

It has mainly been tested with Elasticsearch 6.8 and 7.4.
@getong
Copy link

getong commented Dec 24, 2019

@werbitzky Do you have time to review this pr? We really need it.

@fmcgeough
Copy link

I'd be happy to help test this if you need help. @werbitzky

@evuez
Copy link
Collaborator

evuez commented Feb 7, 2020

Sorry it took me so long to reply.

@reachfh I'm not sure if @werbitzky or I would have the time to review that entire PR anytime soon. If you (or anyone who feels like doing that) could split it in 2 PRs, one for the API changes (Search, the new Templates module and Bulk.post_raw) and another one for the "cosmetic" changes (i.e. most of the things under "general code changes") that would be great.

Just one thing regarding the IO to Logger change for deprecation warnings, I think it'd be better to use @deprecated instead, to avoid spamming the logs (or the console as is currently the case).

Just to be clear, I really appreciate the effort that's been put into this PR and would love to merge it; but I just don't have the time to go through 4000+ line changes to review it.

@NickHath
Copy link

NickHath commented Aug 6, 2020

I am interested in helping with this, specifically upgrading elastix to be compatible with elasticsearch 7.X.

@reachfh
Copy link
Author

reachfh commented Aug 7, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants