Skip to content

Bump jackson to 2.18.4 and fabric8 to 7.2.0#18013

Merged
kgyrtkirk merged 13 commits intoapache:masterfrom
capistrant:jackson-and-fabric8-dep-vers-bump
Jun 19, 2025
Merged

Bump jackson to 2.18.4 and fabric8 to 7.2.0#18013
kgyrtkirk merged 13 commits intoapache:masterfrom
capistrant:jackson-and-fabric8-dep-vers-bump

Conversation

@capistrant
Copy link
Copy Markdown
Contributor

@capistrant capistrant commented May 16, 2025

Description

Getting to Fabric8 7.x requires jackson to be updated (ref #17913). I targeted jackson 2.18.x since jackson 2.19 is brand new. Fabric 8 7.3.0 (also brand new) depends on jackson 2.19, so I stuck with 7.2. Surprisingly ran into only a single compile issue related to bumping jackson.

The jaxb excludes are added because #17370 replaced javax bind with jakarta bind and not excluding it leads to our static checks getting cranky about used undefined deps and unused defined deps. As I understand it, both packages still use the javax package naming and only differ in maven coordinates as a part of the efforts to smooth javax --> jakarta migration.

Testing

  • integration-tests-ex
    • a handful of test groups
    • a long lived docker cluster with MMs where I did batch ingest of wiki data and some sql native queries
  • integration-tests
    • hdfs deep storage

Release note


Key changed/added classes in this PR
  • JodaStuff

This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • a release note entry in the PR description.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added or updated version, license, or notice information in licenses.yaml
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • added integration tests.
  • been tested in a test Druid cluster.

@capistrant
Copy link
Copy Markdown
Contributor Author

I'm trying to figure out why jacoco says I don't have line coverage on the first of the 2 changed lines in JodaStuff... the debugger definitely hits it when I set a break. Found this chatting with copilot.

This happens because JaCoCo tracks coverage at the bytecode level, not source lines. When a line always throws an exception, JaCoCo may not mark it as covered, even if your debugger hits it. This is a known limitation with JaCoCo and exception-throwing lines.

Copy link
Copy Markdown
Member

@kgyrtkirk kgyrtkirk left a comment

Choose a reason for hiding this comment

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

awesome! thank you!

might worth to ban jaxb-api under bannedDependencies in the root pom.xml

note: I think the exclusions could be done via dependencyManagement ; but since with import exclusions doesn't work - that will require to re-declare these 2 in the root pom...not much cleaner....

@capistrant
Copy link
Copy Markdown
Contributor Author

Thanks for the review @kgyrtkirk! I am trying out the explicit ban in root pom

@github-actions github-actions Bot added Area - Batch Ingestion Area - MSQ For multi stage queries - https://github.com/apache/druid/issues/12262 labels Jun 16, 2025
@capistrant
Copy link
Copy Markdown
Contributor Author

I'm currently running the IndexHadoop ITs locally since those are disabled in CI. I want to ensure we are not breaking hadoop integrations by upgrading jackson

@capistrant
Copy link
Copy Markdown
Contributor Author

I'm currently running the IndexHadoop ITs locally since those are disabled in CI. I want to ensure we are not breaking hadoop integrations by upgrading jackson

Confirmed that index_hadoop is able to run with jackson 2.18.4 through these local index_hadoop tasks against hadoop 3.3.6

@kgyrtkirk
Copy link
Copy Markdown
Member

I've noticed that some artifacts by chance that for example commons-lang3 is bound to a lower version than a dep would pull in....and after enabling the corresponding enforcer plugin I've found a lot of downgraded deps...but after a few rounds it also realized that jackson is outdated :) (https://github.com/kgyrtkirk/druid/tree/disallow-downgrades)

would you like to do any more things before merging this?

what would it take to run the IndexHadoop test in the ci ? we could probably create a label or something to run it - so that we don't need to hassle with it manually (if it passes) ...

@capistrant
Copy link
Copy Markdown
Contributor Author

what would it take to run the IndexHadoop test in the ci ? we could probably create a label or something to run it - so that we don't need to hassle with it manually (if it passes) ...

CI for hadoop used to be turned on (at least I think I remember that it was), so I assume all it would take is some conditional set on a label existing to run or not. I think the result of the dev list discussion on future of java11 and hadoop support plays into how worthwhile it would be to try and add back.

Short of baking it in a cluster that is more real than our ITs or my local docker cluster, I don't know of any other things that would be worth doing before this goes in

@kgyrtkirk kgyrtkirk merged commit 691ea3c into apache:master Jun 19, 2025
75 checks passed
@kgyrtkirk
Copy link
Copy Markdown
Member

thank you @capistrant for upgrading these! :)
I'll follow up with some minor ones!

Comment thread server/pom.xml
<dependency>
<groupId>com.fasterxml.jackson.jaxrs</groupId>
<artifactId>jackson-jaxrs-json-provider</artifactId>
<exclusions>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there a reason we did not push the exclusion to the main pom.
cc @capistrant @kgyrtkirk ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I believe it was lost in the cracks..should have been better to do that - but I wanted to get it in since it was open for a month; and it was really usefull for my other dep update PR

there is also some note about snakeyaml being set to 1.33; maybe that could also be taken care of together

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

My bad on this. Let me try to address ASAP.

cryptoe added a commit to cryptoe/druid that referenced this pull request Jun 23, 2025
…x IGNORE_DUPLICATE_MODULE_REGISTRATIONS. Disabling this feature in case multiple modules are registered with the same name.
cryptoe added a commit that referenced this pull request Jun 23, 2025
…RE_DUPLICATE_MODULE_REGISTRATIONS. Disabling this feature in case multiple modules are registered with the same name. (#18167)
@capistrant capistrant added this to the 34.0.0 milestone Jul 22, 2025
kravii pushed a commit to acceldata-io/druid that referenced this pull request Oct 24, 2025
basapuram-kumar pushed a commit to acceldata-io/druid that referenced this pull request Oct 29, 2025
* Bump jackson to 2.14.1 and fabric8 to 7.2.0 (apache#18013)

(cherry picked from commit 691ea3c)

* Fix typo

* Removal of quidem-ut

---------

Co-authored-by: Lucas Capistrant <capistrant@users.noreply.github.com>
prabhjyotsingh pushed a commit to acceldata-io/druid that referenced this pull request Oct 29, 2025
* Bump jackson to 2.14.1 and fabric8 to 7.2.0 (apache#18013)

(cherry picked from commit 691ea3c)

* Fix typo

* Removal of quidem-ut

---------

Co-authored-by: Lucas Capistrant <capistrant@users.noreply.github.com>
(cherry picked from commit 4ce81c1)
kravii pushed a commit to acceldata-io/druid that referenced this pull request Mar 19, 2026
prabhjyotsingh pushed a commit to acceldata-io/druid that referenced this pull request Mar 19, 2026
basapuram-kumar pushed a commit to acceldata-io/druid that referenced this pull request Apr 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area - Batch Ingestion Area - Dependencies Area - Ingestion Area - MSQ For multi stage queries - https://github.com/apache/druid/issues/12262 jacoco:skip Kubernetes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants