You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "xiangfu0 (via GitHub)" <gi...@apache.org> on 2026/04/26 05:02:23 UTC

[PR] Bypass Dependabot for ClassLoaderTest's pinned commons-io 2.11.0 fixture [pinot]

xiangfu0 opened a new pull request, #18335:
URL: https://github.com/apache/pinot/pull/18335

   ## Summary
   
   `ClassLoaderTest` pins `commons-io 2.11.0` as a test fixture — it asserts the literal jar filename `commons-io-2.11.0.jar` and depends on that exact version's contents to validate plugin realm isolation. The same coordinate is also legitimately consumed at a newer version in the top-level [pom.xml](https://github.com/apache/pinot/blob/master/pom.xml), so a repo-wide Dependabot ignore rule is not viable: it would block real commons-io upgrades.
   
   With the previous `<artifactItem>` declaration in [pinot-spi/pom.xml](https://github.com/apache/pinot/blob/master/pinot-spi/pom.xml), Dependabot kept opening PRs that bumped the pinned 2.11.0 reference (e.g. https://github.com/apache/pinot/pull/18331, opened the day after https://github.com/apache/pinot/pull/18308 merged the legitimate `commons-io.version` upgrade in root pom). Each PR required manual revert of the pinot-spi fixture portion. This change makes Dependabot stop scanning the pinned reference altogether.
   
   ## Approach
   
   Refactor the fixture fetch to a two-step pipeline that hides the coordinate from Dependabot's Maven updater while preserving Maven's normal resolution semantics:
   
   1. **`maven-dependency-plugin:get`** with `<artifact>commons-io:commons-io:2.11.0</artifact>` — resolves the jar through Maven's standard resolution chain (local cache → configured mirrors → authenticated repos → SHA-1 checksum validation). Dependabot's Maven updater scans structured `<groupId>`/`<artifactId>`/`<version>` triples inside `<dependency>`/`<plugin>`/`<artifactItem>` blocks; it does not parse single-string `<artifact>` values in plugin configuration. Wide range of ASF projects (Calcite, Drill, etc.) use this same pattern for similar test fixtures.
   2. **`maven-antrun-plugin:run`** copies the resolved jar from `${settings.localRepository}/commons-io/commons-io/2.11.0/commons-io-2.11.0.jar` into `target/test-classes/plugins/assemblybased-pinot-plugin/`, preserving the literal filename that `ClassLoaderTest.assemblyBasedRealm` asserts on via `CodeSource.getLocation().getPath().endsWith("commons-io-2.11.0.jar")`.
   
   Why not simpler alternatives:
   - **Direct HTTPS download** (Ant `<get src="https://repo1.maven.org/...">`): bypasses local cache, configured mirrors, authenticated proxies, and SHA-1 validation. Breaks offline / mirrored / air-gapped builds.
   - **Property-referenced version**: Dependabot follows property references back to coordinates and would still open bump PRs.
   - **`<dependency:get>` with `<destination>`**: the parameter was removed in modern `maven-dependency-plugin` — `dependency:get` only writes to local repo now.
   - **Repo-wide `commons-io` ignore in dependabot.yml**: would block legitimate commons-io upgrades for the production version in root pom.xml.
   
   Also updates [.github/dependabot.yml](https://github.com/apache/pinot/blob/master/.github/dependabot.yml) comment to drop the now-obsolete caveat about commons-io.
   
   ## Test plan
   
   - [x] `./mvnw -pl pinot-spi -am -Dtest=ClassLoaderTest test` — all 5 ClassLoaderTest cases pass (`assemblyBasedRealm`, `classRealms`, `classicPluginClassloader`, `limitedPluginRealm`, `unlimitedPluginRealm`).
   - [x] Verified clean resolution: deleted `~/.m2/repository/commons-io/commons-io/2.11.0/` and `pinot-spi/target/`, then re-ran the test — `dependency:get` re-resolved the jar through Maven, antrun copied it, all tests pass.
   - [x] `./mvnw -pl pinot-spi spotless:apply checkstyle:check license:format license:check` — all clean.
   - [x] Two rounds of independent code review (initial `<get src="https://...">` approach was rejected for bypassing Maven resolution / checksum verification; revised approach approved).
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Bypass Dependabot for ClassLoaderTest's pinned commons-io 2.11.0 fixture [pinot]

Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 merged PR #18335:
URL: https://github.com/apache/pinot/pull/18335


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Bypass Dependabot for ClassLoaderTest's pinned commons-io 2.11.0 fixture [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #18335:
URL: https://github.com/apache/pinot/pull/18335#discussion_r3143065895


##########
pinot-spi/pom.xml:
##########


Review Comment:
   Consider modifying all of the pinned version



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Bypass Dependabot for ClassLoaderTest's pinned commons-io 2.11.0 fixture [pinot]

Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #18335:
URL: https://github.com/apache/pinot/pull/18335#discussion_r3143170349


##########
pinot-spi/pom.xml:
##########


Review Comment:
   Done in 074e944. All five pinned ClassLoaderTest fixtures (`pinot-dropwizard`, `pinot-yammer:shaded`, `pinot-yammer` for the unpack, `metrics-core`, plus the previously-handled `commons-io`) now go through the same `dependency:get` + antrun copy/unzip pipeline. The `<artifactItem>` blocks and the `>= <next>` ignore rules in `.github/dependabot.yml` are gone. Verified locally with a wiped local cache for all four fixture coordinates — all 5 ClassLoaderTest cases pass.



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Bypass Dependabot for ClassLoaderTest's pinned commons-io 2.11.0 fixture [pinot]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #18335:
URL: https://github.com/apache/pinot/pull/18335#issuecomment-4321371334

   ## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/18335?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 55.34%. Comparing base ([`aad170a`](https://app.codecov.io/gh/apache/pinot/commit/aad170afaa58a64d8d3ca3f4c6aba207beca1109?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`0f08e74`](https://app.codecov.io/gh/apache/pinot/commit/0f08e74051fc280790ea3e8960360299a3dd8be6?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   > :exclamation:  There is a different number of reports uploaded between BASE (aad170a) and HEAD (0f08e74). Click for more details.
   > 
   > <details><summary>HEAD has 16 uploads less than BASE</summary>
   >
   >| Flag | BASE (aad170a) | HEAD (0f08e74) |
   >|------|------|------|
   >|java-21|5|1|
   >|unittests|2|1|
   >|temurin|5|1|
   >|unittests2|1|0|
   >|integration|3|0|
   >|integration1|1|0|
   >|integration2|1|0|
   >|custom-integration1|1|0|
   ></details>
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #18335      +/-   ##
   ============================================
   - Coverage     63.38%   55.34%   -8.04%     
   + Complexity     1658      837     -821     
   ============================================
     Files          3252     2542     -710     
     Lines        198637   147285   -51352     
     Branches      30765    23729    -7036     
   ============================================
   - Hits         125897    81520   -44377     
   + Misses        62667    58747    -3920     
   + Partials      10073     7018    -3055     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/pinot/pull/18335/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/18335/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [integration](https://app.codecov.io/gh/apache/pinot/pull/18335/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [integration1](https://app.codecov.io/gh/apache/pinot/pull/18335/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [integration2](https://app.codecov.io/gh/apache/pinot/pull/18335/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [java-21](https://app.codecov.io/gh/apache/pinot/pull/18335/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.34% <ø> (-8.04%)` | :arrow_down: |
   | [temurin](https://app.codecov.io/gh/apache/pinot/pull/18335/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.34% <ø> (-8.04%)` | :arrow_down: |
   | [unittests](https://app.codecov.io/gh/apache/pinot/pull/18335/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.34% <ø> (-8.03%)` | :arrow_down: |
   | [unittests1](https://app.codecov.io/gh/apache/pinot/pull/18335/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.34% <ø> (-0.01%)` | :arrow_down: |
   | [unittests2](https://app.codecov.io/gh/apache/pinot/pull/18335/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/18335?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org