-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[es] upgrade to master #5656
[es] upgrade to master #5656
Conversation
jenkins, test it |
5372fc2
to
be358d8
Compare
@spalger is this still actively being worked on? Is there anything I could do to help move it along? Getting this PR merged would be great for the feature/ingest branch which relies on features in ES master. |
since es 3.0 removed it, and we are trying to upgrade... needs to go!
This reverts commit 6f73fc0dbb1609637c16f4fae9b282adfd3fc262.
This reverts commit f68235db55a6d11dea1ef145446a75cc24cf84e2.
This reverts commit 904d0e2e9b31b380e4e39b36d11c50b85286dbd4.
This reverts commit dc4dec2.
This reverts commit 5a3b9d402c844ec32ea48a98e68893cb137abc30.
a008cad
to
7aee4a5
Compare
(cherry picked from commit f4e3226)
The changes look good and all, but I'd like to run this against master and verify functionality myself before I give you a seal of approval. Unfortunately, the configuration I need to run ES is currently broken in master, so I'm waiting on fixes before I can test this out. |
I killed this jenkins job because it had a bunch of failures and was beginning to time out. |
Getting the following error when trying to load Discover with a scripted field added to the index pattern:
|
Other than the above error, I haven't seen any glaring issues. |
@@ -28,8 +28,8 @@ module.exports = function ({ Plugin }) { | |||
cert: string(), | |||
key: string() | |||
}).default(), | |||
apiVersion: string().default('2.0'), | |||
engineVersion: string().valid('^2.1.0').default('^2.1.0') | |||
apiVersion: Joi.string().default('master'), |
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.
How does the elasticsearch js client interpret master
being passed as the API version? Wouldn't we still want to pin each version of Kibana to a specific version of the ES API?
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.
Well, since the elasticsearch installation is pinned to master we need to use the same API version. The API in use in es master isn't shipping in any other version. "master" is the only option.
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.
Maybe I missed a conversation about how all this is going to be handled. Is the plan to replace master everywhere with the correct version of ES when we cut a release?
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.
We'd replace master with a specific version whenever we created the version branch, which generally happens at feature freeze.
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.
Got it, makes sense
Fixes #5649
Fixes #5658
Breaking Changes doc: https://www.elastic.co/guide/en/elasticsearch/reference/master/breaking-changes-3.0.html
{ script, lang }
under the script key - 904d0e2