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

tests: include Elasticsearch 5 in Travis-CI #132

Merged
merged 3 commits into from
Dec 5, 2017

Conversation

hjhsalo
Copy link
Contributor

@hjhsalo hjhsalo commented Nov 22, 2017

  • include Elasticsearch5 in Travis-CI tests.

Signed-off-by: Harri Hirvonsalo harri.hirvonsalo@cern.ch

@hjhsalo hjhsalo self-assigned this Nov 22, 2017
@hjhsalo hjhsalo removed their assignment Nov 22, 2017
@hjhsalo hjhsalo force-pushed the 103_es5_for_travis branch 10 times, most recently from e21936f to a031868 Compare November 23, 2017 10:52
@hjhsalo hjhsalo changed the title WIP tests: include Elasticsearch 5 in Travis-CI tests: include Elasticsearch 5 in Travis-CI Nov 23, 2017
@hjhsalo
Copy link
Contributor Author

hjhsalo commented Nov 23, 2017

ES5 tests in Travis are failing but I think otherwise the tasks defined in #103 are done, so I guess this is ready for review?

If the reviewer has knowledge about .travis.yml, could you suggest a way to reduce the repetion of ES_DOWNLOAD_URL=https://download.elasticsearch.org/elasticsearch/release/org/elasticsearch/distribution/tar/elasticsearch/${ES_VERSION}/elasticsearch-${ES_VERSION}.tar.gz to something like ES_DOWNLOAD_URL=ES_2_DOWNLOAD_URL? The same also applies to download URL of ES5.
I couldn't come up with solutions by myself...

@hjhsalo
Copy link
Contributor Author

hjhsalo commented Nov 23, 2017

Please note that files https://github.com/inveniosoftware/invenio/blob/master/scripts/provision-elasticsearch.sh and https://github.com/inveniosoftware/invenio/blob/master/elasticsearch/Dockerfile should probably be prepared for ES5.

Maybe something like this could be used in Dockerfile and then change behaviour of provision-elasticsearch.sh based on value of $ES_VERSION:

ARG ES_VERSION=2
ENV ES_VERSION ${ES_VERSION:-2}

FROM elasticsearch:$ES_VERSION
...

Also note that mapper-attachment -plugin is deprecated in ES5 in favor of ingest-attachment -plugin and that name of the binary that is used to install plugins changes in ES5 as well ($ elasticsearch-plugin install ... VS $ plugin install ...)

https://www.elastic.co/guide/en/elasticsearch/plugins/5.6/mapper-attachments.html
https://www.elastic.co/guide/en/elasticsearch/plugins/5.6/ingest-attachment.html

@hjhsalo hjhsalo force-pushed the 103_es5_for_travis branch 2 times, most recently from 3df5d09 to 7260991 Compare November 23, 2017 13:37
@nharraud nharraud self-assigned this Nov 23, 2017
@nharraud
Copy link
Member

The current state of this PR does not enable to support elasticsearch 5.

  • index init fails because the mappings provided by invenio-marc21 are not elasticsearch 5 compatible (see the breaking changes)
  • The percolator API changed and breaks the tests (see the breaking changes)

Currently invenio-search uses all the mappings provided. However if we want to support both elasticsearch-5 and elasticsearch-2 we would need to provide two different set of mappings. Thus we would need to refactor invenio-search.

@inveniosoftware/architects Another issue pointed out by @slint : Elasticsearch version 2 End Of Life is 2018-02-28. We might want to go for version 5 or 6 immediately and drop version 2.

@slint
Copy link
Member

slint commented Nov 23, 2017

There's also an official migration helper plugin that goes over some basic checks of the cluster. I've added the output of running the tool on the relevant issue #103

@lnielsen lnielsen assigned slint and unassigned nharraud Nov 27, 2017
@hjhsalo hjhsalo assigned hjhsalo and slint and unassigned slint and hjhsalo Nov 27, 2017
@slint slint unassigned slint and hjhsalo Dec 1, 2017
@slint slint force-pushed the 103_es5_for_travis branch 2 times, most recently from 8a3dbf7 to ffe3dfb Compare December 1, 2017 14:00
from invenio_indexer.api import RecordIndexer
from invenio_search import current_search, current_search_client

from .models import OAISet
from .proxies import current_oaiserver
from .query import query_string_parser

# ES5 percolator
PERCROLATOR_DOC_TYPE = '.percolators' if ES_VERSION[0] == 2 else 'percolators'
Copy link
Member

Choose a reason for hiding this comment

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

PERCROLATOR -> PERCOLATOR

* include Elasticsearch5 in Travis-CI tests.

Signed-off-by: Harri Hirvonsalo <harri.hirvonsalo@cern.ch>
@slint slint force-pushed the 103_es5_for_travis branch 2 times, most recently from f387a5c to c71f4ee Compare December 1, 2017 17:27
@lnielsen lnielsen self-assigned this Dec 5, 2017
Signed-off-by: Alexander Ioannidis <a.ioannidis@cern.ch>
Co-authored-by: Harri Hirvonsalo <harri.hirvonsalo@cern.ch>
@lnielsen lnielsen merged commit 051e8f4 into inveniosoftware:master Dec 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants