-
Notifications
You must be signed in to change notification settings - Fork 202
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
Upgrade jackson to 2.15.4 #17
Conversation
@jun-he Can you please review this change? Thank you 👍 |
@varunu28 thanks for the contribution! I will review it ASAP. |
@@ -46,8 +46,8 @@ allprojects { | |||
resolutionStrategy { | |||
// Force 2.11.4 until upgrade is done | |||
// Jackson is to be locked due to conversion issue with Double<>BigDecimal | |||
force "com.fasterxml.jackson.core:jackson-databind:2.11.4" | |||
force "com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:2.11.4" | |||
force "com.fasterxml.jackson.core:jackson-databind:2.15.4" |
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.
It would be better to remove the pin here. But the dependency of conductor might block it. There is an ongoing effort to remove the conductor from the Maestro (#5).
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.
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.
Thanks @varunu28 for the change. I think this incremental upgrade is better than the current pin to 2.11.4
. I will run some tests on the workflow data to see if there is any serde or conversion issue.
@varunu28 doing the load test right now and will get back to you soon. |
Load test is done and the change looks good. |
fc18574
to
87c66d8
Compare
@jun-he I ran the write-lock. Can you please do another review? |
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.
Thanks for the change. LGTM.
This addresses #10 .
Doing an incremental upgrade from the current version i.e.
2.11.4
, the latest version at which build is successful is2.15.4
https://mvnrepository.com/artifact/com.fasterxml.jackson.core/jackson-core
Upgrading to the next version i.e.
2.16.0
results in test failures with below error log: