Skip to content

Conversation

@rjernst
Copy link
Member

@rjernst rjernst commented Mar 17, 2020

Re-applies the change from #53523 along with test fixes.

rjernst and others added 4 commits March 16, 2020 11:54
In Jackson 2.9 SMILE handles floats directly instead of expecting
doubles.

relates #53523
closes #53626
We had updated the test when upgrading jackson because yaml now supports
binary. But json still doesn't. This updates the test to reflect that.

Closes #53624
Jackson's YAML parser does weirdness with a custom base64 decoding when trying to "read" text (using
`.text()` or `.textOrNull()`) from a YAML byte array. Because of this, we need to skip comparing
these fields with `.textOrNull`, instead using the correct `.binaryValue()` comparison.

Resolves #53622
@rjernst rjernst added >non-issue :Core/Infra/Core Core issues without another label v8.0.0 v7.7.0 labels Mar 17, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (:Core/Infra/Core)

With the upgrade from Jackson 2.8.11 to 2.10.3 there is no longer a
need to Base64 encode BytesArray for comparing YAML serializations.

Fixes #53625
Copy link
Contributor

@jakelandis jakelandis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rjernst
Copy link
Member Author

rjernst commented Mar 17, 2020

@elasticmachine run elasticsearch-ci/2
@elasticmachine run elasticsearch-ci/default-distro

@rjernst rjernst merged commit d63cda1 into master Mar 17, 2020
@rjernst rjernst deleted the jackson_upgrade branch March 17, 2020 17:26
rjernst added a commit that referenced this pull request Mar 17, 2020
Re-applies the change from #53523 along with test fixes.

closes #53626
closes #53624
closes #53622
closes #53625

Co-authored-by: Nik Everett <[email protected]>
Co-authored-by: Lee Hinman <[email protected]>
Co-authored-by: Jake Landis <[email protected]>
rjernst added a commit that referenced this pull request Mar 17, 2020
This fixes fallout from a bad backport of #53642
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants