Skip to content
This repository has been archived by the owner on Sep 12, 2024. It is now read-only.

Support jackson 2.11.0+ #1301

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

jwiklund
Copy link

@jwiklund jwiklund commented Aug 6, 2020

Jackson 2.11.0 changes the default _tzSerializedWithColon value from false to true,
this causes a "Id hash mismatch" when trying to deploy a job.
withColonInTimeZone has been available since 2.9.1 so this will break on older version,
If this is an issue we would probably have to resort to reflection.

Jackson 2.11.0 changes the default _tzSerializedWithColon value from false to true.
This causes a "Id hash mismatch" when trying to deploy a job.

withColonInTimeZone has been available since 2.9.1 so this will break on older version.
If this is an issue we would probably have to resort to reflection.
@jwiklund jwiklund requested a review from a team as a code owner August 6, 2020 13:15
@codecov
Copy link

codecov bot commented Aug 6, 2020

Codecov Report

Merging #1301 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master    #1301      +/-   ##
============================================
+ Coverage     53.02%   53.04%   +0.01%     
- Complexity     1657     1658       +1     
============================================
  Files           269      269              
  Lines         13135    13138       +3     
  Branches       1505     1505              
============================================
+ Hits           6965     6969       +4     
+ Misses         5656     5655       -1     
  Partials        514      514              

@davidxia
Copy link
Contributor

davidxia commented Aug 6, 2020

Just curious what you need newer jackson versions for. Is it to fix this vuln? https://github.com/spotify/helios/network/alert/pom.xml/com.fasterxml.jackson.core:jackson-databind/open

@jwiklund
Copy link
Author

jwiklund commented Aug 6, 2020

@davidxia We have a mixed repository with both apollo and scio (to test the combination) and the newest scio 0.9.2 requires jackson 2.11.0.

@davidxia
Copy link
Contributor

davidxia commented Aug 6, 2020

Are you able to do some shading for just your service? Helios is in maintenance mode and rolling this out will affect the entire fleet and take a while.

@jwiklund
Copy link
Author

jwiklund commented Aug 6, 2020

I have copied this Json file into our repository which seems to fix the problem for us. I guess we can keep this open if someone else encounters the same problem to avoid pushing this out.

@davidxia
Copy link
Contributor

davidxia commented Aug 6, 2020

oh nice, which JSON file are you referring to?

@jwiklund
Copy link
Author

jwiklund commented Aug 6, 2020

I mean the Json.java file in this repository, I applied this PR's change and have it in our repository and it loads before the Json class from helios-client.jar :)

@jwiklund
Copy link
Author

jwiklund commented Aug 6, 2020

Since it's only used for testing it's kind of okay :)

@davidxia
Copy link
Contributor

davidxia commented Aug 6, 2020

Whoa, what dark magic is this?

@jwiklund
Copy link
Author

jwiklund commented Aug 6, 2020

Well pre modules everything is a flat namespace so java will load the first file that matches the name (and local files are placed before jar files by maven), I'm assuming if we turn on module support this way of hacking stuff will stop working but it's convenient when needed (and very annoying when it's done by misstake and you have to figure out why that class you can clearly see have a function does not).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants