Skip to content

fix(user_tools): download explicit tools jar - #2108

Open
WilliamK112 wants to merge 2 commits into
NVIDIA:devfrom
WilliamK112:codex/download-explicit-tools-jar
Open

fix(user_tools): download explicit tools jar#2108
WilliamK112 wants to merge 2 commits into
NVIDIA:devfrom
WilliamK112:codex/download-explicit-tools-jar

Conversation

@WilliamK112

@WilliamK112 WilliamK112 commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Summary

  • distinguish an explicitly supplied --tools_jar from the default wheel-provided tools JAR
  • download/localize an explicit tools JAR before constructing the Java classpath, including remote gs:// values
  • preserve the direct local-resource fast path when no explicit tools JAR is supplied
  • cover both the default-resource and explicit-remote paths with regression tests

Root cause

When use_local_tools_jar() was true, _process_jar_arg() used the current JAR path directly without checking whether the user had supplied --tools_jar. A remote JAR URL could therefore flow into the Java classpath unchanged, causing Java to fail to load the tools main class.

Fixes #1889.

Validation

  • PYTHONPATH=src python -m pytest -q tests/spark_rapids_tools_ut/test_path_boundary.py (12 passed)
  • PYTHONPATH=src python -m pytest -q tests/spark_rapids_tools_ut/test_tool_argprocessor.py -k test_with_platform_with_eventlogs_with_jar_files (12 passed, 127 deselected)
  • python -m pylint -d fixme --load-plugins pylint_pydantic --rcfile=../.pylintrc src/spark_rapids_pytools/rapids/rapids_tool.py tests/spark_rapids_tools_ut/test_path_boundary.py (10.00/10)
  • python -m flake8 src/spark_rapids_pytools/rapids/rapids_tool.py tests/spark_rapids_tools_ut/test_path_boundary.py
  • python -m py_compile src/spark_rapids_pytools/rapids/rapids_tool.py tests/spark_rapids_tools_ut/test_path_boundary.py
  • git diff --check upstream/dev...HEAD

Signed-off-by: WilliamK112 <164879897+WilliamK112@users.noreply.github.com>
@github-actions github-actions Bot added the user_tools Scope the wrapper module running CSP, QualX, and reports (python) label Jun 23, 2026
…cit-tools-jar

Signed-off-by: WilliamK112 <164879897+WilliamK112@users.noreply.github.com>
@WilliamK112
WilliamK112 marked this pull request as ready for review August 22, 2026 18:59
@greptile-apps

greptile-apps Bot commented Sep 9, 2026

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

The PR appears safe to merge; the changed branch correctly localizes explicit remote JARs while retaining the default resource fast path.

Summary

  • Preserves direct use of the default local resource JAR.
  • Routes explicit JAR locations through the existing storage download path.
  • Adds regression coverage for implicit local and explicit gs:// paths.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Read wrapper toolsJar] --> B{Explicit value supplied?}
    B -- No --> C[Resolve default RAPIDS JAR]
    C --> D{Local wheel resource?}
    D -- Yes --> E[Use resource path directly]
    D -- No --> F[Download into work directory]
    B -- Yes --> F
    F --> G[Use localized path]
    E --> H[Set jarFilePath and jarFileName]
    G --> H
Loading

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

Labels

user_tools Scope the wrapper module running CSP, QualX, and reports (python)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] CLI errors out when using Remote URL for Tools JAR

2 participants