Skip to content
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

Update to jackson 2.8.1 #18939

Merged
merged 1 commit into from
Aug 5, 2016
Merged

Update to jackson 2.8.1 #18939

merged 1 commit into from
Aug 5, 2016

Conversation

tlrx
Copy link
Member

@tlrx tlrx commented Jun 17, 2016

Jackson 2.8.1 has been released. This PR updates the version to the latest version of Jackson which fix #18076 and is also more strict when it build objects.

@tlrx tlrx added :Core/Infra/Core Core issues without another label v5.0.0-alpha4 labels Jun 17, 2016
JsonStreamContext context = base.getOutputContext();
if (context instanceof JsonWriteContext) {
((JsonWriteContext) context).writeValue();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

should we put an assertions that it actually is a JsonWriteContext?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure

@tlrx
Copy link
Member Author

tlrx commented Jul 8, 2016

2.8.0 has been released: https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.8

@tlrx
Copy link
Member Author

tlrx commented Aug 2, 2016

@tlrx tlrx added review and removed WIP labels Aug 2, 2016
@tlrx tlrx changed the title Update to jackson 2.8.0 Update to jackson 2.8.1 Aug 2, 2016
@tlrx
Copy link
Member Author

tlrx commented Aug 3, 2016

@s1monw Do you want to have another look?

assertEquals("\"responses\"[{\"error\":{\"root_cause\":[{\"type\":\"illegal_state_exception\",\"reason\":\"foobar\"}],\"type\":\"illegal_state_exception\",\"reason\":\"foobar\"},\"status\":500},{\"error\":{\"root_cause\":[{\"type\":\"illegal_state_exception\",\"reason\":\"baaaaaazzzz\"}],\"type\":\"illegal_state_exception\",\"reason\":\"baaaaaazzzz\"},\"status\":500}]",
builder.endObject();

assertEquals("{\n" +
Copy link
Contributor

Choose a reason for hiding this comment

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

pretty printing is difficult, not sure if it works on windows with trailing whiltespaces / new lines etc?

@s1monw
Copy link
Contributor

s1monw commented Aug 3, 2016

left some comments @tlrx thanks for doing this

@tlrx
Copy link
Member Author

tlrx commented Aug 3, 2016

@s1monw Thanks for your review! I removed all pretty printings.

@tlrx
Copy link
Member Author

tlrx commented Aug 5, 2016

Rally benchmarks shows no performance regression. I'd like to merge it soon - can I go @s1monw ?

@s1monw
Copy link
Contributor

s1monw commented Aug 5, 2016

LGTM

This commit updates Jackson to the 2.8.1 version, which is more strict when it comes to build objects. It also adds the snakeyaml dependency that was previously shaded in jackson libs.

It also closes elastic#18076
@tlrx tlrx force-pushed the update-to-jackson-2.8.0 branch from 787ed6a to 841d5a2 Compare August 5, 2016 10:26
@tlrx tlrx merged commit 841d5a2 into elastic:master Aug 5, 2016
@tlrx tlrx removed the review label Aug 5, 2016
@tlrx tlrx deleted the update-to-jackson-2.8.0 branch August 5, 2016 10:28
@tlrx
Copy link
Member Author

tlrx commented Aug 5, 2016

Thanks @s1monw !

dadoonet added a commit to dadoonet/elasticsearch that referenced this pull request Aug 9, 2016
@wgorder
Copy link

wgorder commented Aug 26, 2016

Great need this one too +1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Core Core issues without another label >upgrade v5.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid JSON Being Returned
4 participants