-
-
Notifications
You must be signed in to change notification settings - Fork 238
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
switch projects to use elasticsearch 5.x #25
Conversation
projects/france/docker-compose.yml
Outdated
container_name: pelias_elasticsearch | ||
restart: always | ||
environment: [ "discovery.type=single-node" ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it possible for us to set this as a default in the Dockerfile itself? It would be ideal to not have to specify it everywhere including in the schema documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can try, actually I wanted to mention this setting to you, apparently, bash doesn't support exporting vars with a .
in them (try it!) so it'll have to be done in either elasticsearch.yml
or via a flag for the elasticsearch binary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yikes, that's fun. I'm sure we can figure out a solution :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh yea it's easy, I'll publish and remove the references to it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved via rebase
We are testing this with
Some other warning:
|
Yes, those can't be fixed (yet) because they would break compatibility with ES2. But one of the great new features of ES5 (over ES2 where it exists but is hard to use) is that the deprecation log is enabled by default. So when the time comes to upgrade to ES6 we will be well informed :) |
Okay :) I set up our pelias server with es5 (api.staging.pelias.jawg.io) and es2.4 (api.pelias.jawg.io) versions. The UI is here https://pelias.jawg.io/ I will do some tests tomorrow ;-) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Results of Acceptance tests for France:
ES2.4:
Aggregate test results
Pass: 283
Improvements: 11
Fail: 107
Placeholders: 0
Regressions: 264
Took 31463ms
Test success rate 59.63%
ES5:
Aggregate test results
Pass: 285
Improvements: 11
Fail: 107
Placeholders: 0
Regressions: 262
Took 28237ms
Test success rate 59.94%
The main difference is in autocomplete for San Francisco and that's not very significant here.
Waiting for pelias/schema#323 now :)
FYI we decided not to merge all of pelias/schema#323 for now, so we pulled out the 'good bits' in to pelias/schema#328 and merged that to |
There has been some confusion after we switched the Docker images to use a non-root user. It's now fairly easy for permissions to be set incorrectly. This line in the example script should make things right.
@Joxit FYI we are going to start running all the docker projects on We're going to hold off doing our |
Okay, I'll also see from our side if we will upgrade our If we do, I'll inform you if we have any issues 😉 |
switch projects to use elasticsearch 5.x
Depends on pelias/schema#323, please merge that first and wait for a new version of the
pelias/schema:latest
branch to be published to dockerhub.Many projects are also based off the experimental
pelias/schema:portland-synonyms
image, so that image will need to pull in master after 323 is merged and then republished to dockerhub.