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

travis: elasticsearch v5 support #75

Merged
merged 6 commits into from Dec 4, 2017
Merged

Conversation

lnielsen
Copy link
Member

@lnielsen lnielsen commented Nov 30, 2017

No description provided.

@lnielsen lnielsen changed the title WIP travis: elasticsearch v5 support travis: elasticsearch v5 support Dec 1, 2017
@lnielsen lnielsen removed their assignment Dec 1, 2017
@lnielsen lnielsen changed the title travis: elasticsearch v5 support WIP travis: elasticsearch v5 support Dec 1, 2017
@slint slint self-assigned this Dec 1, 2017
@lnielsen lnielsen changed the title WIP travis: elasticsearch v5 support travis: elasticsearch v5 support Dec 1, 2017
@lnielsen lnielsen self-assigned this Dec 1, 2017
.travis.yml Outdated
- "mkdir /tmp/elasticsearch"
- "wget -O - $ES_URL | tar xz --directory=/tmp/elasticsearch --strip-components=1"
- "/tmp/elasticsearch/bin/elasticsearch &"
- "sleep 20"
- "travis_retry pip install --upgrade pip setuptools py"
- "travis_retry pip install twine wheel coveralls requirements-builder"
- "requirements-builder -e all --level=min setup.py > .travis-lowest-requirements.txt"
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't it be "requirements-builder -e all -e $EXTRAS --level=min setup.py > .travis-lowest-requirements.txt"

"testrecord-v1.0.0": {
"properties": {
"control_number": {
"type": "string"
Copy link
Member

Choose a reason for hiding this comment

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

This is ok with ES5, but fires a warning. I'm not sure if we should explicitly change ES5 mappings to use the correct field type for string (text, unless there was a index: not_analyzed, in which case it should be keyword)

setup.py Outdated
@@ -45,13 +45,21 @@
'redis>=2.10.0',
]

invenio_search_version = '1.0.0b2'
Copy link
Member

Choose a reason for hiding this comment

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

1.0.0b3 when inveniosoftware/invenio-search#123 is merged

"testrecord-v1.0.0": {
"properties": {
"control_number": {
"type": "string"
Copy link
Member

Choose a reason for hiding this comment

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

should be "type": "keyword"

"type": "string"
},
"description": {
"type": "string"
Copy link
Member

Choose a reason for hiding this comment

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

"type": "text"

"type": "long"
},
"title": {
"type": "string",
Copy link
Member

Choose a reason for hiding this comment

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

"type": "text"

"copy_to": "suggest_title"
},
"type": {
"type": "string"
Copy link
Member

@nharraud nharraud Dec 1, 2017

Choose a reason for hiding this comment

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

"type": "keyword"

@lnielsen lnielsen removed their assignment Dec 1, 2017
@slint slint removed their assignment Dec 1, 2017
@slint
Copy link
Member

slint commented Dec 1, 2017

@lnielsen FYI, I pushed a fix for the failing builds (the bulk index queue needed to be initialized in the doctests before sending records for bulk indexing)

lnielsen and others added 4 commits December 1, 2017 18:12
* Moves mappings into v2/v5 subdirectories.

* Extends travis with v2/v5 versions.

* Moves invenio-search to extra dependencies with support for ES 2/5.
* Removes automatic indexing on models_committed signal since this
  method does not allow enough control over the indexing process and
  the method will have unintended side effects if enabled with modules
  that are unaware of the signal being install. Controllers should
  instead explicitly index records when needed.
@lnielsen lnielsen merged commit 1f8b5ac into inveniosoftware:master Dec 4, 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

4 participants