Skip to content

[FEA] Consume renamed cudf-spark-jni artifacts [databricks] - #15929

Open
NvTimLiu wants to merge 2 commits into
NVIDIA:mainfrom
NvTimLiu:jni-artifacatId-rename
Open

[FEA] Consume renamed cudf-spark-jni artifacts [databricks]#15929
NvTimLiu wants to merge 2 commits into
NVIDIA:mainfrom
NvTimLiu:jni-artifacatId-rename

Conversation

@NvTimLiu

@NvTimLiu NvTimLiu commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Part of #15882.

Description

Update cuDF Spark to consume JNI artifacts under their renamed project identity.

This change:

  • introduces cudf-spark-jni.version and cudf-spark-jni.artifactId properties
  • uses the artifact ID property for Maven dependencies, shading exclusions, and distribution unpacking
  • reads cudf-spark-jni-version-info.properties for JNI version discovery
  • updates dependency-discovery and nightly-build controls
  • synchronizes the generated Scala 2.13 POMs
  • updates related documentation and review configuration

Centralizing the artifact ID follows the existing cudf-spark-private dependency pattern and allows the artifact coordinate to be overridden without editing every module.

AI assistance disclosure: Codex assisted with validation and drafting this description. The author reviewed the complete diff and final description before PR creation.

Checklists

Documentation

  • Updated for new or modified user-facing features or behaviors
  • No user-facing change

Testing

  • Added or modified tests to cover new code paths
  • Covered by existing tests
    (Please provide the names of the existing tests in the PR description.)
  • Not required

Performance

  • Tests ran and results are added in the PR description
  • Issue filed with a link in the PR description
  • Not required

Performance testing is not required because this change only updates dependency coordinates, build metadata, documentation, and the filename used to discover existing JNI version metadata. It does not modify Spark execution or GPU processing paths.

Update cuDF Spark to consume JNI artifacts under their new project identity.

- add cudf-spark-jni artifact ID and version properties
- resolve Maven dependencies, shading exclusions, and unpack steps through the artifact ID property
- load the renamed cudf-spark-jni version metadata resource
- update dependency discovery, nightly controls, Scala 2.13 POMs, documentation, and review configuration

Centralizing the artifact ID matches the cudf-spark-private dependency pattern and allows the coordinate to be overridden without editing each module.

Signed-off-by: timl <timl@nvidia.com>
@NvTimLiu
NvTimLiu requested review from a team as code owners September 9, 2026 02:15
@NvTimLiu NvTimLiu added the build Related to CI / CD or cleanly building label Sep 9, 2026
@greptile-apps

greptile-apps Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

The PR appears safe to merge, with no new actionable findings and no outstanding previous finding.

Summary

  • Centralizes the JNI artifact ID and version in Maven properties.
  • Updates module dependencies, shading exclusions, distribution unpacking, snapshot discovery, and nightly-build controls.
  • Synchronizes the Scala 2.12 and Scala 2.13 build metadata.
  • Updates JNI version-resource discovery and related documentation.
  • No changes were made after the previous review.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  P["Root Maven properties<br/>artifactId + version"] --> M["Module dependencies"]
  P --> S["Aggregator shading exclusions"]
  P --> D["Distribution JNI unpacking"]
  P --> C["Snapshot/nightly build controls"]
  D --> J["cudf-spark-jni JAR"]
  J --> R["cudf-spark-jni-version-info.properties"]
  R --> V["Runtime version and duplicate-JAR discovery"]
Loading

Comment thread sql-plugin/src/main/scala/com/nvidia/spark/rapids/Plugin.scala
@NvTimLiu
NvTimLiu requested review from nvauto and a lite review from Copilot September 9, 2026 03:31
@NvTimLiu NvTimLiu self-assigned this Sep 9, 2026

Copilot AI left a comment

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.

🟡 Changes recommended

One runtime behavior gap remains around detecting mixed legacy/new JNI JARs on the classpath, and a few updated scripts/docs still contain inconsistencies that should be corrected.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates cudf-spark to consume the renamed cudf-spark-jni Maven artifacts by centralizing the JNI artifact coordinates (artifactId + version) and propagating that change through module dependencies, shading/distribution packaging, dependency-discovery scripts, and related documentation/configuration.

Changes:

  • Introduces/uses ${cudf-spark-jni.version} and ${cudf-spark-jni.artifactId} across Maven modules (including generated Scala 2.13 POMs).
  • Updates runtime/version metadata discovery to use cudf-spark-jni-version-info.properties and refreshes user-facing docs accordingly.
  • Adjusts CI/support scripts and review/config metadata to reflect the renamed JNI project.
File summaries
File Description
udf-compiler/pom.xml Switches JNI dependency to ${cudf-spark-jni.artifactId}.
tests/pom.xml Switches JNI dependency to ${cudf-spark-jni.artifactId}.
sql-plugin/src/main/scala/com/nvidia/spark/rapids/RapidsConf.scala Updates allowMultipleJars docs to mention cudf-spark-jni.
sql-plugin/src/main/scala/com/nvidia/spark/rapids/Plugin.scala Renames JNI version-info resource filename and updates multi-jar detection label.
sql-plugin/pom.xml Switches JNI dependency to ${cudf-spark-jni.artifactId}.
sql-plugin-fileio/pom.xml Switches JNI dependency to ${cudf-spark-jni.artifactId}.
sql-plugin-columnar/pom.xml Switches JNI dependency to ${cudf-spark-jni.artifactId}.
sql-plugin-api/pom.xml Switches JNI dependency to ${cudf-spark-jni.artifactId}.
shuffle-plugin/pom.xml Switches JNI dependency to ${cudf-spark-jni.artifactId}.
scala2.13/udf-compiler/pom.xml Mirrors JNI artifactId property usage in Scala 2.13 POM.
scala2.13/tests/pom.xml Mirrors JNI artifactId property usage in Scala 2.13 POM.
scala2.13/sql-plugin/pom.xml Mirrors JNI artifactId property usage in Scala 2.13 POM.
scala2.13/sql-plugin-fileio/pom.xml Mirrors JNI artifactId property usage in Scala 2.13 POM.
scala2.13/sql-plugin-columnar/pom.xml Mirrors JNI artifactId property usage in Scala 2.13 POM.
scala2.13/sql-plugin-api/pom.xml Mirrors JNI artifactId property usage in Scala 2.13 POM.
scala2.13/shuffle-plugin/pom.xml Mirrors JNI artifactId property usage in Scala 2.13 POM.
scala2.13/pom.xml Adds ${cudf-spark-jni.version} + ${cudf-spark-jni.artifactId} and updates dep mgmt + build-info arg.
scala2.13/iceberg-common/pom.xml Switches JNI dependency to ${cudf-spark-jni.artifactId} (Scala 2.13).
scala2.13/dist/pom.xml Updates dist unpack dependency to ${cudf-spark-jni.artifactId} (Scala 2.13).
scala2.13/api_validation/pom.xml Updates provided JNI dependency to ${cudf-spark-jni.artifactId} (Scala 2.13).
scala2.13/aggregator/pom.xml Updates shade exclusion to ${cudf-spark-jni.artifactId} (Scala 2.13).
pom.xml Adds ${cudf-spark-jni.version} + ${cudf-spark-jni.artifactId} and updates dep mgmt + build-info arg.
jenkins/spark-nightly-build.sh Updates documentation comment for overriding JNI version property.
iceberg-common/pom.xml Switches JNI dependency to ${cudf-spark-jni.artifactId}.
docs/additional-functionality/advanced_configs.md Updates allowMultipleJars documentation to mention cudf-spark-jni.
dist/pom.xml Updates dist unpack dependency to ${cudf-spark-jni.artifactId}.
build/make-scala-version-build-files.sh Updates Scala POM sync anchors to the renamed JNI version property tag.
api_validation/README.md Updates dependency text to cudf-spark-jni.
api_validation/pom.xml Updates provided JNI dependency to ${cudf-spark-jni.artifactId}.
aggregator/pom.xml Updates shade exclusion to ${cudf-spark-jni.artifactId}.
.greptile/rules.md Updates upstream-dependency wording to cudf-spark-jni.
.greptile/config.json Updates Greptile upstream repo references and guidance text for JNI rename.
.github/workflows/mvn-verify-check/get-deps-sha1.sh Updates dependency-discovery script to use renamed JNI artifact/version property.
Review details
  • Files reviewed: 31/33 changed files
  • Comments generated: 7
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/workflows/mvn-verify-check/get-deps-sha1.sh Outdated
Comment thread sql-plugin/src/main/scala/com/nvidia/spark/rapids/Plugin.scala
Comment thread .greptile/config.json Outdated
Comment thread aggregator/pom.xml Outdated
Comment thread docs/additional-functionality/advanced_configs.md Outdated
Comment thread scala2.13/aggregator/pom.xml Outdated
Comment thread sql-plugin/src/main/scala/com/nvidia/spark/rapids/RapidsConf.scala Outdated
Resolve the cudf-spark-jni artifact ID from Maven in the dependency hash lookup so command-line property overrides are honored.

Update stale spark-rapids-jni and spark-rapids-private references in the aggregator shading comments and Greptile guidance. Keep the generated Scala 2.13 aggregator POM synchronized.

Correct the allowMultipleJars configuration description typo in RapidsConf and the generated advanced configuration documentation.

Signed-off-by: timl <timl@nvidia.com>

Copilot AI left a comment

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.

🔵 Needs a closer look

A few renamed references are still inconsistent (e.g., dist unpack execution naming/comments and the runtime “report this issue” repo URL), which should be corrected to avoid misleading build/debug output and users.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

sql-plugin/src/main/scala/com/nvidia/spark/rapids/Plugin.scala:86

  • SPARK_RAPIDS_REPO_URL still points to the legacy repository URL (NVIDIA/spark-rapids), but this repo has been renamed and the message at runtime will direct users to the wrong place for support/bug reports. Update the URL to NVIDIA/cudf-spark to match the new project identity.
  • Files reviewed: 31/33 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@NvTimLiu

NvTimLiu commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

@greptileai review

Copilot AI left a comment

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.

🟡 Changes recommended

The dist unpack sections now use the new ${cudf-spark-jni.artifactId} but still contain spark-rapids-jni in nearby comments/execution ids, which should be updated for consistency and maintainability.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 31/33 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI left a comment

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.

🟡 Changes recommended

A few remaining user-facing/build-maintenance strings in the modified files still reference the legacy spark-rapids-jni naming and should be aligned to avoid confusion during troubleshooting and build debugging.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 31/33 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread sql-plugin/src/main/scala/com/nvidia/spark/rapids/Plugin.scala
@NvTimLiu NvTimLiu changed the title [FEA] Consume renamed cudf-spark-jni artifacts [reduced-it] [FEA] Consume renamed cudf-spark-jni artifacts [databricks] Sep 9, 2026

@amahussein amahussein left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I found no blocking issues and would approve this change. No Blossom premerge build for #15929 appeared in the last 200 builds, so I verified the two cross-repository assumptions directly: Maven Central currently publishes com.nvidia:cudf-spark-jni:26.10.0-SNAPSHOT with the cuda12 classifier, and the published JAR contains cudf-spark-jni-version-info.properties. The PR's checksum script also resolves the published JNI SHA-1. The mixed legacy/new JAR detection gap is already covered by #15937, so I am not reopening it here. I left one non-blocking inline coordination question.

Comment thread pom.xml
@amahussein

Copy link
Copy Markdown
Collaborator

build

@NvTimLiu

Copy link
Copy Markdown
Collaborator Author

pre-merge build PASS as expected: spark-rapids-jni --> cudf-spark-jni

333ff179-ff52-40d6-b10f-e56baf03451f

@NvTimLiu
NvTimLiu requested a review from sameerz September 10, 2026 07:57
@NvTimLiu
NvTimLiu requested review from a team, GaryShen2008 and gerashegalov September 10, 2026 07:57
@NvTimLiu NvTimLiu added the feature request New feature or request label Sep 10, 2026

@amahussein amahussein left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Related to CI / CD or cleanly building feature request New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants