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.

Sign up for GitHub

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

Jump to bottom

elasticsearch 7 #394

Closed
wants to merge 3 commits into from
Closed

elasticsearch 7 #394

wants to merge 3 commits into from

Conversation

missinglink
Copy link
Member

@missinglink missinglink commented Nov 19, 2019 •
edited by orangejulius
Loading

es7 compatibility (breaks compatibility with es5)

notable changes:

todo:

  • decide on whether we remove word_delimiter or move it down below the synonyms
  • squash the commits and/or add a breaking change commit message
  • update pelias/config to set the default schema.typeName to _doc (done in Elasticsearch 7 Support pelias#831)
  • update pelias/model to use the _doc type instead of doc (see above)
  • update all the importers to use the updated pelias/model version (also no longer relevant)

@missinglink missinglink force-pushed the es7 branch 5 times, most recently from ae73ad3 to 7d81d11 Compare November 19, 2019 16:17
@orangejulius
Copy link
Member

orangejulius commented Nov 19, 2019 •
edited
Loading

Regarding the word_delimiter filter, see also #392

@missinglink missinglink force-pushed the es7 branch 18 times, most recently from 253fb75 to 3e5d39e Compare November 20, 2019 15:49
@missinglink
Copy link
Member Author

build passing! woohoo! compatibility dance 🕺 👯

@missinglink
Copy link
Member Author

missinglink commented Nov 21, 2019 •
edited
Loading

So.. while this schema works great to create an index on ES6/ES7 the importers all fail due to this line in pelias/model which assumes the type is named doc instead of _doc.

It's kinda unfortunate but correct to do it like this, we must continue with the type named doc for as long as we support ES5 (due to the lack of support for underscores in the type name for v5).

Once we drop backwards support for 5 we can go ahead and change that line in pelias/model, it, unfortunately, makes this hard to test in the meantime.

@missinglink missinglink mentioned this pull request Nov 22, 2019
Closed
@orangejulius orangejulius mentioned this pull request Nov 22, 2019
Closed
23 tasks
@missinglink missinglink force-pushed the es7 branch 6 times, most recently from a0f602f to 1d1b43a Compare November 27, 2019 10:53
@missinglink missinglink mentioned this pull request Nov 27, 2019
Closed
4 tasks
@missinglink missinglink force-pushed the es7 branch 2 times, most recently from 317b802 to 936174a Compare November 27, 2019 14:23
orangejulius added a commit that referenced this pull request Jan 7, 2020
@orangejulius
feat(integration): Add total hits helper
8200d27 
In Elasticsearch 7+, the [hits count is now an
object](https://www.elastic.co/guide/en/elasticsearch/reference/current/breaking-changes-7.0.html#hits-total-now-object-search-response).

This was needed because Elasticsearch now includes a performance
improvement that allows non-exact hit counts to be used when the exact
count isn't needed.

This adds a helper to wrap around the breaking change and support either
the old or new format.

Extracted from #394
Connects pelias/pelias#831
@orangejulius orangejulius mentioned this pull request Jan 7, 2020
Merged
@orangejulius
Copy link
Member

I've just rebased this branch against master after extracting the hits helper to a separate PR. The list of changes is getting pretty small, and after #418 I bet we can even remove more.

orangejulius added a commit that referenced this pull request Jan 8, 2020
@orangejulius
chore(integration): Move schema creation options to common config
642b160 
After looking at the changes in
#394, I noticed that all the
parameters used to create test indices during the integration tests were
identical.

Because we will want to make changes to those options as part of our
move to support Elasticsearch 7, this PR is a pure refactoring that
moves those options to the `common` object passed to each integration
test.

This should allow us to make any future changes to index creation
options in a single place.
orangejulius added a commit that referenced this pull request Jan 8, 2020
@orangejulius
chore(integration): Move schema creation options to common config
839caf9 
After looking at the changes in
#394, I noticed that all the
parameters used to create test indices during the integration tests were
identical.

Because we will want to make changes to those options as part of our
move to support Elasticsearch 7, this PR is a pure refactoring that
moves those options to the `common` object passed to each integration
test.

This should allow us to make any future changes to index creation
options in a single place.
This was referenced Jan 8, 2020
Merged
Merged
@orangejulius
Copy link
Member

All aspects of this PR have now been extracted into or otherwise handled by the following PRs:

Referring to this PR and splitting it up into component pieces was a super useful exercise in making sure we have all our ES7 bases covered. And now we do! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@missinglink @orangejulius