-
Notifications
You must be signed in to change notification settings - Fork 369
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 8.x upgrade #917
ES 8.x upgrade #917
Conversation
spec.add_dependency 'activesupport', '>= 5.2' # Remove with major version bump, 8.x | ||
spec.add_dependency 'elasticsearch', '>= 7.12.0', '< 7.14.0' | ||
spec.add_dependency 'activesupport', '>= 6.1' | ||
spec.add_dependency 'elasticsearch', '>= 8.11', '< 9.0' |
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 the plan here to do a major version bump of Chewy as well? So the chewy 7.x series will continue to get any bugfixes for ES7 gems, and the chewy 8.x series will support the ES8.x gems?
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 the plan here to do a major version bump of Chewy as well?
Yes, since the major version of the Chewy, is equal to major version of the ES.
So the chewy 7.x series will continue to get any bugfixes for ES7 gems, and the chewy 8.x series will support the ES8.x gems?
This my plan, but since I have never done this before, cannot say for sure, how hard it will be to support both versions at the same time.
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.
Hopefully it shouldn't be too much work to keep the 7.x
branch up to date with changes in the Elasticsearch Ruby client. The last minor release is 7.17
and we're not planning to release any new minors or add any new features for the moment, just addressing bugs or security issues, so patch releases only.
8bed864
to
d189dde
Compare
'type' => 'mapper_parsing_exception', | ||
'reason' => 'object mapping for [object] tried to parse field [object] as object, but found a concrete value' | ||
'type' => 'document_parsing_exception', | ||
'reason' => '[1:27] object mapping for [object] tried to parse field [object] as object, but found a concrete value' |
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.
Are these numeric prefixes ([1:27]
in this example) new error-code constants from the ES client gem ... or contingent on the spec data setup here, or something else?
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.
Don't know this yet, I've just made all specs passing, to see how many things do we break by upgrading to ES 8.x
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.
When can we expect for this to be considered for release?
This seems blocked -- are you open to PRs that try to nudge portions of this forward, or you want to handle yourself? |
Hey, I started some work in #962, would love an approval of a workflow (there will be some fun with extracting passwords and certs from docker container) |
in favour of #962 |
Replace this text with a summary of the changes in your PR.
The more detailed you are, the better.
Before submitting the PR make sure the following are checked:
[Fix #issue-number]
(if the related issue exists).master
(if not - rebase it).