FrankChen021 commented on PR #20371: URL: https://github.com/apache/druid/pull/20371#issuecomment-5724845595
This is an automated review by Codex GPT-5.6 Luna(Max). ## Compatibility analysis Dependency under review: `org.apache.maven.resolver:maven-resolver-impl`, source `1.3.1`, target `2.0.23`. The reserved and currently verified head is `2a94814ef6780db4df00672eac59b42e86c9a9d7`; the base commit is `2fed7ca11d9852412ce76798a3fa150ddae9afdb`. The PR diff is one line in `services/pom.xml`, changing only `maven-resolver-impl` from `1.3.1` to `2.0.23`; no Druid production or test source is changed. The complete published Maven Central inventory for `maven-resolver-impl` from the source through the target is 73 versions: `1.3.1`, `1.3.2`, `1.3.3`, `1.4.0`, `1.4.1`, `1.4.2`, `1.6.1`, `1.6.2`, `1.6.3`, `1.7.0`, `1.7.1`, `1.7.2`, `1.7.3`, `1.8.0`, `1.8.1`, `1.8.2`, `1.9.0`, `1.9.1`, `1.9.2`, `1.9.4`, `1.9.5`, `1.9.6`, `1.9.7`, `1.9.8`, `1.9.10`, `1.9.11`, `1.9.12`, `1.9.13`, `1.9.14`, `1.9.15`, `1.9.16`, `1.9.17`, `1.9.18`, `1.9.19`, `1.9.20`, `1.9.21`, `1.9.22`, `1.9.23`, `1.9.24`, `1.9.25`, `1.9.26`, `1.9.27`, `2.0.0-alpha-1`, `2.0.0-alpha-2`, `2.0.0-alpha-3`, `2.0.0-alpha-5`, `2.0.0-alpha-6`, `2.0.0-alpha-7`, `2.0.0-alpha-8`, `2.0.0-alpha-10`, `2.0.0-alpha-11`, `2.0.0`, `2.0.1`, `2.0.2`, `2.0.3`, `2.0.4`, `2.0.5`, `2.0.6`, `2.0.7`, `2.0.8`, `2.0.9`, `2.0.10`, `2.0.11`, `2.0.13`, `2.0.14`, `2.0.15`, `2.0.16`, `2.0.17`, `2.0.18`, `2.0.20`, `2.0.21`, `2.0.22`, and `2.0.23`. `1.9.3`, `1.9.9`, `2.0.0-alpha-4`, `2.0.0-alpha-9`, `2.0.12`, and `2.0.19` are absent from the published M aven Central metadata and were not treated as releases. The 1.x chain was reviewed through `1.9.27`; the alpha bridge and every published 2.0.x patch release were reviewed through `2.0.23`. The 2.0.0 release notes document the `ServiceLocator` removal, session/bootstrap changes, transport restructuring, and the move from `java.io.File` to NIO paths. The 2.0.23 release notes add repository-key, tracking, checksum, redirect/TLS, policy, atomic-publication, lock, coordinate-validation, and remote-string-sanitization changes. The cumulative target effect is therefore a major resolver bootstrap/transport/session and local-repository behavior migration, not a compatible implementation-only patch. Compatibility decisions: | Category | Decision | Evidence | | --- | --- | --- | | API/ABI | INCOMPATIBLE | `DefaultServiceLocator` is present in the published 1.3.1 implementation jar and absent from the published 2.0.23 implementation jar. | | Runtime | INCOMPATIBLE | The unchanged Druid bootstrap cannot compile; the target also requires the new supplier/transport arrangement. | | Configuration | CONCERN | Druid's `MavenRepositorySystemUtils.newSession()` and proxy/local-repository setup need an intentional 2.0 session-builder migration. | | Serialization/wire | SAFE for Druid data/query formats | The one-line PR changes no Druid serialization or query wire classes; Maven repository metadata/HTTP behavior is covered under clients/runtime. | | Persistence | CONCERN | `pull-deps` writes a local Maven repository, while the 2.0 transition changes path/session/locking and the target hardens tracking, checksum, publication, and repository-key handling. | | Clients | INCOMPATIBLE for the pull-deps repository client | Druid imports the 1.x `org.eclipse.aether.transport.http.HttpTransporterFactory`; no `maven-resolver-transport-http:2.0.23` artifact is published, and 2.0 uses replacement transport modules. | | Transitive dependencies | INCOMPATIBLE | Enforcer reports direct `api`, `spi`, `util`, connector, and transport modules at `1.3.1` alongside `impl:2.0.23`; `impl:2.0.23` also requires `maven-resolver-named-locks`. | | Licenses | CONCERN | Resolver artifacts are Apache-2.0, but `licenses.yaml` still records the old 1.3.1 resolver modules and Maven 3.6.0 provider; a real migration adds supplier, named-lock, replacement transport, and Maven-provider metadata to review. | | Extension/plugin SPI | INCOMPATIBLE | `PullDependencies.java` and `PullDependenciesTest.java` directly use the removed locator and old connector/transport bootstrap APIs on the extension download path. | Overall compatibility verdict: INCOMPATIBLE. A safe repair requires coordinated dependency alignment, a replacement supplier/session bootstrap, transport selection, provider/Maven model alignment, proxy/local-repository validation, license metadata, server packaging updates, and focused tests. That is a genuinely large and risky migration for this one-line Dependabot PR, so it is not safe to repair or approve in this round. ## Druid impact The PR changes only `services/pom.xml` at the resolver implementation version. The affected unchanged call sites are `services/src/main/java/org/apache/druid/cli/PullDependencies.java` imports and `getRepositorySystem()` / `getRepositorySystemSession()` (the removed locator, old HTTP transporter, Maven session helper, proxy selector, and local repository manager), plus the equivalent real bootstrap in `services/src/test/java/org/apache/druid/cli/PullDependenciesTest.java`. `server/pom.xml` still declares resolver connector/transport/provider at the old versions and marks them as used dependencies for packaging. `licenses.yaml` still has the old resolver/provider entries. No tracked Druid source or test code changed in the PR. ## Validation - `git diff --check` passed for the complete PR diff. - Published-jar inspection confirmed `DefaultServiceLocator` in `maven-resolver-impl:1.3.1` and absent in `maven-resolver-impl:2.0.23`. - Published-POM inspection confirmed `maven-resolver-impl:2.0.23` depends on the 2.0.23 API/SPI/util/named-locks set; the target supplier module brings the replacement connector, file/Apache transports, and Maven resolver provider, while the old HTTP transport GAV returns HTTP 404 at 2.0.23. - All 20 failed job logs were inspected. The compile-failure jobs report `PullDependencies.java:[45,31]` and `[204,5]` cannot find `org.eclipse.aether.impl.DefaultServiceLocator`; the four static jobs report Maven Enforcer `RequireUpperBoundDeps` failures for the mixed 1.3.1/2.0.23 resolver graph. - Attached artifacts were inspected: one web-check diagnostics artifact and 13 unit-report artifacts (about 75 MiB); the reports contain no independent JUnit `<failure>`/`<error>` result, and the small diagnostics artifacts are heap/jstack captures. ## CI gate At the pre-closure refresh the PR was OPEN, non-draft, `MERGEABLE` with `UNSTABLE` merge state, exact head `2a94814ef6780db4df00672eac59b42e86c9a9d7`, 27 CheckRuns, 0 StatusContexts, and rollup `FAILURE`. Five CheckRuns succeeded: PR title, triage, CodeQL JavaScript, CodeQL Python, and actions-timeline. The 20 failures were: - CodeQL: `Analyze (java)` — the `druid-services` compile break. - Static Checks: `openrewrite`, `strict-compilation`, `packaging-check (25) / packaging-check-jdk25`, and `static-checks-maven` — the mixed resolver dependency graph fails upper-bound convergence. - Static Checks: `web-checks` — the same `druid-services` compile break. - Unit & Integration: `docker-tests / Run Docker tests`, `QTest 0/4`, `QTest 1/4`, `QTest 2/4`, `QTest 3/4`, and every JDK25 shard `test-jdk25-[C*]`, `test-jdk25-[S*]`, `test-jdk25-[K*,U*,Z*,Y*,X*]`, `test-jdk25-[N*,Q*]`, `test-jdk25-[I*,L*,J*]`, `test-jdk25-[M*,A*,V*,W*]`, `test-jdk25-[E*,G*,F*]`, `test-jdk25-[R*,B*,P*]`, and `test-jdk25-[H*,D*,T*,O*]` — the same compile break before those suites could run. The remaining non-success items were `coverage-jacoco` SKIPPED, because prerequisites failed, and the aggregate CodeQL CheckRun NEUTRAL. These are not eligible grounds for approval; the rollup is not green. ## Automation actions No CI reruns were issued: every completed failure was deterministic and PR-caused, so rerunning would not be evidence-backed; the rerun count remains 0/2 for every failed job on this exact head. No worktree source repair, commit, push, or approval was made. This PR is being closed as `CLOSED_INCOMPATIBLE` / high-effort migration, with the evidence above preserved for a future dedicated Resolver 2 migration. No merge was performed. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
