Align all usages of Jackson to be 2.14.0 #91725
Conversation
jakelandis
added
>upgrade
:Core/Infra/REST API
|
Hi @jakelandis, I've created a changelog YAML for you. |
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
|
@ChrisHegarty can you take a quick look for anything I may have missed specific to 2.14.0 ? |
|
@elasticsearchmachine run elasticsearch-ci/part-1 |
|
I don't get what is going on with CI ... the first run was success, the second run had a few failures due dependency verification issues ( due to no longer listing multiple elder versions hashes), the third run had a single failure. None of the failures will reproduce locally. @elasticsearchmachine run elasticsearch-ci/part-1 |
There was a problem hiding this comment.
In general I think we should be moving away from this global version declaration style. It makes upgrading very difficult, mostly because of third party libs that often have complicated reasons they can't be upgraded. We should upgrade simple uses aggressively (eg x-content, where it matters most for security), and 3rd party libs as necessary/possible.
| @@ -10,15 +10,13 @@ apply plugin: 'elasticsearch.java' | |||
|
|
|||
| archivesBaseName = "x-content-impl" | |||
|
|
|||
| String jacksonVersion = "2.14.0" | |||
There was a problem hiding this comment.
We had decoupled this specifically so that we don't need to upgrade all version Jackson in parallel. Often is is easier to upgrade x-content here (since it is direct use), but for difficult to upgrade 3rd party libs that depend on it (eg azure). I think we should keep this as specified separately from the "global" version (and I'm not sure we should have the global version anymore, we should just be pulling in the version 3rd party libs depend on unless there is a reason to force it higher).
There was a problem hiding this comment.
Agreed - my preference is to retain this x-content specific version. The ability to upgrade the version of Jackson independent of other parts of the system is a feature here (not a bug).
| @@ -27,8 +27,6 @@ versions << [ | |||
| 'azureCommon': '12.15.1', | |||
| 'azureCore': '1.27.0', | |||
| 'azureCoreHttpNetty': '1.11.9', | |||
| 'azureJackson': '2.13.2', | |||
| 'azureJacksonDatabind': '2.13.2.2', | |||
There was a problem hiding this comment.
There was a reason this had to be separate...but I don't remember what it was. Does 2.14.0 match what azure depends on, for databind as well?
There was a problem hiding this comment.
these artifacts depend oncom.fasterxml.jackson.dataformat » jackson-dataformat-xml 2.13.4
https://mvnrepository.com/artifact/com.azure/azure-storage-blob/12.20.1
https://mvnrepository.com/artifact/com.azure/azure-storage-common/12.19.1
however azure-core depends on
com.fasterxml.jackson.core » jackson-databind 2.13.4.2
com.fasterxml.jackson.dataformat » jackson-dataformat-xml 2.13.4
com.fasterxml.jackson.datatype » jackson-datatype-jsr310 2.13.4
|
@elasticmachine update branch |
|
merge conflict between base and head |
|
I will close this PR and replace it with an upgrade to 2.14.2 |
related: #90553
Note - I do not plan to back port this to 7.x since #90553 was not back ported and am not confident such a large upgrade across the board for 7.x is wise.