-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Fix BWC issues for x_pack/usage #55181
Conversation
Fixes several BWC issues in x_pack/usage discovered after introduction of the bwc test. Relates to elastic#54847
Pinging @elastic/es-analytics-geo (:Analytics/Aggregations) |
Looks like there are still broken piece we serialize back to 7.x. I will continue digging. |
@elasticmachine update branch |
Thanks @imotov for handling this and sorry that you ran into it. To me the approach seems okay -- I find it cleanest to just re-introduce the classes as you've done here. As a small note, perhaps we could mark the classes In the PR where I naively removed |
Yes! Marked them as deprecated. I wonder if we should add a test that would fail if the version is 9.0 and Flattened is still there.
I checked info and it looks like it's all local. The only thing that goes over the wire are sting maps of strings and boolean. So, I think we should be fine. But I have added a test for it just in case I missed something there. |
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.
The enrich part looks good. Thanks for fixing @imotov.
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.
Looks good to me too.
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.
LGTM3! Thanks again for tackling all of this usage stuff :)
Relates elastic#55378 Relates elastic#55181
Fixes several BWC issues in x_pack/usage discovered after introduction of the
bwc test.
Relates to #54847
@jtibshirani and @martijnvg adding you to reviewers since I had to add enrich and some parts for flatten back.
Even though master no longer reports flatten as a feature we still can receive it from 7.x nodes during upgrade. I wasn't sure if I should add these piece back or just hard code flatten in the reader, read a string, two booleans and an int and be done with it. Please, let me know what you think.