Packaging tools jar with the python package #1634

sayedbilalbari · 2025-04-14T18:47:32Z

This PR enables changes for packaging the tools jar with the python whl file. This avoids downloading the latest jar from mvn each time any cli command is run.
This packages the jar from source and bundles it in the wheel in the folder tools_resources.

This changes the build process by packaging the jar as part of the wheel in all build scenarios. In case of a fat build, it also packages the csp-resources, csp-resources.tgz along with the tools jar.

The user_tools expects the tools jar now as part of the resources. In case not available, it still falls back to downloading it from maven

The changes have been tested locally using the --platform onprem. Since there are no platform specific changes needed, it should be sufficient.

This pull request includes changes to the user_tools/build.sh script and related files to add support for a "non-fat" mode, which only downloads necessary resources instead of all dependencies. Additionally, it introduces a new folder structure to separate tools resources from CSP resources.

Build script enhancements:

user_tools/build.sh: Added a new variable TOOLS_RESOURCE_FOLDER to define the tools resources folder and updated the download_web_dependencies function to handle both fat and non-fat modes. [1] [2]
user_tools/build.sh: Modified the build function to incorporate the new build modes and ensure the appropriate resources are downloaded and organized.

Python tool context updates:

user_tools/src/spark_rapids_pytools/rapids/tool_ctxt.py: Introduced a new class variable tools_resource_path to manage tools resources and updated methods to handle the new folder structure and non-fat mode. [1] [2] [3] [4] [5] [6]

Resource manager script changes:

user_tools/src/spark_rapids_pytools/resources/dev/prepackage_mgr.py: Added support for the fetch_all_csp flag to control whether all CSP resources are downloaded or just the tools jar. Updated the script to handle the new tools resources directory. [1] [2] [3] [4] [5] [6]

Other minor changes:

user_tools/pyproject.toml: Added *.jar to the package data to ensure the tools jar is included in the package.

Signed-off-by: Sayed Bilal Bari <[email protected]>

amahussein

I started to look at the changes.
Isn't supposed to be a change in the README.md to update the instructions on how to start with the new change?
For example, what is the assumption? If we do a pip install -e . and run, then the jar will be downloaded from mvn at runtime?

amahussein · 2025-04-15T16:41:09Z

user_tools/build.sh

+# prepackage_mgr.py file downloads the dependencies for the csp-related resources
+# in case of fat mode.
+# In case of non-fat mode, it just copies the tools jar into the tools-resources folder
+# --fetch_all_csp=True toggles the fat/non-fat mode for the script


the name fetch_all_csp is not good in that option. If this is a flag indicating Fat mode, then why don't we use something more descriptive like fat_mode_enabled ?

I will update the comments to make this more clear. So previous behavior was -

in case fat passed, then build_jar, download_all_dependencies, compress and add it to the wheel

in case no, then just create the whl from the source files

For this ./build.sh fat vs ./build.sh triggered either cases

Now to replicate the same, script calling behavior , we still call either ./build.sh fat and ./build.sh. The difference now, fat mode packs all the csp related dependencies by compressing it into a tgz file and includes it in the resources.
fetch_all_csp flag is for the internal prepackage_mgr to include the jar or the jar/csp resources. Hence --fetch_all_csp as a param.

Let me know if this still seems wrong, will update it in that case

I did not get the new behavior.
for both build options (fat and non-fat), do the tools-jar go into tools-resources or it depends on the build mode?

For both build modes, the tools-jar goes into the resources.
In the fat mode is that all csp dependencies are also packed with the jar.

amahussein · 2025-04-15T17:46:15Z

The behavior is not correct after the change. Merging the current behavior will break miserably on all pipelines and sites that relies on pip install or python build without calling the build.sh

running the spark_rapids gave me the following error:

2025-04-15 12:40:39,618 INFO rapids.tools.profiling.ctxt: No prepackaged resources found.
2025-04-15 12:40:39,618 INFO rapids.tools.profiling.ctxt: Fetching the Rapids Jar URL from local context
2025-04-15 12:40:39,618 ERROR rapids.tools.profiling: Exception occurred downloading jar None
Traceback (most recent call last):
  File "/work_dir/src/spark_rapids_pytools/rapids/rapids_tool.py", line 434, in _process_jar_arg
    tools_jar_url = self.ctxt.get_rapids_jar_url()
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/work_dir/src/spark_rapids_pytools/rapids/tool_ctxt.py", line 209, in get_rapids_jar_url
    return self._get_tools_jar()
           ^^^^^^^^^^^^^^^^^^^^^
  File "/work_dir/src/spark_rapids_pytools/rapids/tool_ctxt.py", line 251, in _get_tools_jar
    raise ValueError(
ValueError: Tools JAR file name not found in context.Rebuild the wheel package
2025-04-15 12:40:39,618 ERROR rapids.tools.profiling: Failed in processing arguments
2025-04-15 12:40:39,618 ERROR root: Profiling. Raised an error in phase [Process-Arguments]
Traceback (most recent call last):
  File "/work_dir/src/spark_rapids_pytools/rapids/rapids_tool.py", line 128, in wrapper
    func_cb(self, *args, **kwargs)  # pylint: disable=not-callable
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/work_dir/src/spark_rapids_pytools/rapids/rapids_tool.py", line 184, in _process_arguments
    raise ex
  File "/work_dir/src/spark_rapids_pytools/rapids/rapids_tool.py", line 177, in _process_arguments
    self._process_rapids_args()
  File "/work_dir/src/spark_rapids_pytools/rapids/rapids_tool.py", line 632, in _process_rapids_args
    self._process_jar_arg()
  File "/work_dir/src/spark_rapids_pytools/rapids/rapids_tool.py", line 443, in _process_jar_arg
    raise e
  File "/work_dir/src/spark_rapids_pytools/rapids/rapids_tool.py", line 434, in _process_jar_arg
    tools_jar_url = self.ctxt.get_rapids_jar_url()
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/work_dir/src/spark_rapids_pytools/rapids/tool_ctxt.py", line 209, in get_rapids_jar_url
    return self._get_tools_jar()
           ^^^^^^^^^^^^^^^^^^^^^
  File "/work_dir/src/spark_rapids_pytools/rapids/tool_ctxt.py", line 251, in _get_tools_jar
    raise ValueError(
ValueError: Tools JAR file name not found in context.Rebuild the wheel package

The user-tools should do the following:

if the --tools_jar arg is provided, then it should be used.
lookup the jar inside resources
finally, pull the latest jar from mvn that matches the python build.

The above steps guarantee that the pipelines will keep working until we finish upgrading them.

Signed-off-by: Sayed Bilal Bari <[email protected]>

sayedbilalbari · 2025-04-15T21:21:21Z

Thanks @amahussein for the review. Have pushed changes to allow fallback to maven download in case of building from source.
Updating the README file to make this more understandable

sayedbilalbari · 2025-04-15T21:25:28Z

I started to look at the changes. Isn't supposed to be a change in the README.md to update the instructions on how to start with the new change? For example, what is the assumption? If we do a pip install -e . and run, then the jar will be downloaded from mvn at runtime?

Yes, currently only in case of package built using build.sh is when the jar expected to be included in the resource. In case of building from source, the jar is downloaded from mvn. Like you suggested, an alternate to avoid this would be including a github hook that includes the jar file in the resource.
However, I feel keeping the build process separate adds a logical separation. Rather update the instructions and the internal jobs to use the ./build.sh as default rather than building from source using pip

user_tools/src/spark_rapids_pytools/rapids/tool_ctxt.py

amahussein

Thanks @sayedbilalbari !
Can you merge latest dev branch to update the build-version. This might have impact on testing your branch especially when retrieving the jar from mvn.

user_tools/src/spark_rapids_pytools/rapids/tool_ctxt.py

amahussein · 2025-04-16T15:05:56Z

user_tools/build.sh

+# prepackage_mgr.py file downloads the dependencies for the csp-related resources
+# in case of fat mode.
+# In case of non-fat mode, it just copies the tools jar into the tools-resources folder
+# --fetch_all_csp=True toggles the fat/non-fat mode for the script


I did not get the new behavior.
for both build options (fat and non-fat), do the tools-jar go into tools-resources or it depends on the build mode?

amahussein · 2025-04-16T18:15:01Z

user_tools/build.sh

@@ -37,6 +38,8 @@ TOOLS_JAR_FILE=""


 # Function to run mvn command to build the tools jar
+# This function skips the test cases and builds the jar file and only
+# picks the jar file without sources/javadoc/tests..


We need to pass arguments to the build_jar function.
This probably should come from build.sh so that we can have different builds for different jobs.
For example, in #1639 I added a profile-id release. We should be pass that argument down to the mvn command in order to create the specific build we need.

Thanks @amahussein . I am evaluating the final behavior of this script on the basis of the internal build jobs and post having finalised the design, will update the build.sh file to take appropriate params.

Signed-off-by: Sayed Bilal Bari <[email protected]>

amahussein · 2025-04-24T16:05:06Z

Marking this as draft as per offline discussion that the changes are being evaluated by CI/CD internal team first.

sayedbilalbari · 2025-04-28T22:56:20Z

The changes have been tested internally. Waiting on the final check for it.

amahussein

Thanks @sayedbilalbari !
Lets take a second look from @parthosa because he is familiar with the build script.

user_tools/build.sh

Signed-off-by: Sayed Bilal Bari <[email protected]>

sayedbilalbari · 2025-04-30T18:25:45Z

Thanks @parthosa . Have updated with the changes

user_tools/build.sh

Signed-off-by: Sayed Bilal Bari <[email protected]>

parthosa

Thanks @sayedbilalbari. LGTME.

amahussein

Thanks @sayedbilalbari !
LGTME

sayedbilalbari · 2025-05-06T04:54:14Z

The build script has been tested for the fat/non-fat internal CI/CD jobs. Looks good. Merging

sayedbilalbari added 2 commits April 11, 2025 17:56

Changes for building tools jar with the whl file

7385caf

Signed-off-by: Sayed Bilal Bari <[email protected]>

Changes for tools jar in the whl

3874af5

Signed-off-by: Sayed Bilal Bari <[email protected]>

github-actions bot added the user_tools Scope the wrapper module running CSP, QualX, and reports (python) label Apr 14, 2025

sayedbilalbari added 2 commits April 14, 2025 11:50

Updated headers and lint errors

39216a9

Signed-off-by: Sayed Bilal Bari <[email protected]>

Lint errors + python unit test failure correction

ec032bb

Signed-off-by: Sayed Bilal Bari <[email protected]>

sayedbilalbari marked this pull request as ready for review April 14, 2025 22:26

sayedbilalbari requested review from amahussein and parthosa April 14, 2025 22:26

sayedbilalbari changed the title ~~Packaging tools jar with whl file~~ Packaging tools jar with the python package Apr 14, 2025

amahussein reviewed Apr 15, 2025

View reviewed changes

Pushed changes for mvn fallback

2466599

Signed-off-by: Sayed Bilal Bari <[email protected]>

parthosa reviewed Apr 16, 2025

View reviewed changes

user_tools/src/spark_rapids_pytools/rapids/tool_ctxt.py Show resolved Hide resolved

user_tools/src/spark_rapids_pytools/rapids/tool_ctxt.py Show resolved Hide resolved

amahussein requested changes Apr 16, 2025

View reviewed changes

Merge remote-tracking branch 'nv/dev' into issue-1496-bilal

ee24ee3

amahussein reviewed Apr 16, 2025

View reviewed changes

amahussein assigned sayedbilalbari Apr 17, 2025

sayedbilalbari added 5 commits April 17, 2025 14:22

Merge remote-tracking branch 'nv/dev' into issue-1496-bilal

3efc7ea

Merge remote-tracking branch 'nv/dev' into issue-1496-bilal

f5c54b7

Updating build script to take jar URL

104d18a

Signed-off-by: Sayed Bilal Bari <[email protected]>

Updated the script to package downloaded jar

34bdfd8

Signed-off-by: Sayed Bilal Bari <[email protected]>

Added validation to the url passed for downloading the jar

1b740a1

Signed-off-by: Sayed Bilal Bari <[email protected]>

sayedbilalbari requested review from amahussein and parthosa April 23, 2025 00:43

amahussein marked this pull request as draft April 24, 2025 16:05

amahussein removed request for parthosa and amahussein April 24, 2025 16:06

amahussein marked this pull request as ready for review April 29, 2025 18:49

amahussein reviewed Apr 29, 2025

View reviewed changes

sayedbilalbari requested review from parthosa and amahussein April 30, 2025 17:09

parthosa reviewed Apr 30, 2025

View reviewed changes

user_tools/build.sh Outdated Show resolved Hide resolved

user_tools/build.sh Show resolved Hide resolved

user_tools/build.sh Outdated Show resolved Hide resolved

user_tools/build.sh Outdated Show resolved Hide resolved

user_tools/build.sh Show resolved Hide resolved

Review comment changes

0dc45a7

Signed-off-by: Sayed Bilal Bari <[email protected]>

parthosa reviewed Apr 30, 2025

View reviewed changes

user_tools/build.sh Show resolved Hide resolved

Adding http failure flag to curl

b048fa4

Signed-off-by: Sayed Bilal Bari <[email protected]>

parthosa approved these changes Apr 30, 2025

View reviewed changes

amahussein approved these changes May 1, 2025

View reviewed changes

sayedbilalbari merged commit 4f761ad into NVIDIA:dev May 6, 2025
14 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Packaging tools jar with the python package #1634

Packaging tools jar with the python package #1634

sayedbilalbari commented Apr 14, 2025 •

edited

Loading

amahussein left a comment

amahussein Apr 15, 2025

sayedbilalbari Apr 15, 2025 •

edited

Loading

amahussein Apr 16, 2025

sayedbilalbari Apr 16, 2025 •

edited

Loading

amahussein commented Apr 15, 2025 •

edited

Loading

sayedbilalbari commented Apr 15, 2025

sayedbilalbari commented Apr 15, 2025

amahussein left a comment

amahussein Apr 16, 2025

amahussein Apr 16, 2025

sayedbilalbari Apr 18, 2025

amahussein commented Apr 24, 2025 •

edited

Loading

sayedbilalbari commented Apr 28, 2025

amahussein left a comment

sayedbilalbari commented Apr 30, 2025

parthosa left a comment

amahussein left a comment

sayedbilalbari commented May 6, 2025

Packaging tools jar with the python package #1634

Packaging tools jar with the python package #1634

Conversation

sayedbilalbari commented Apr 14, 2025 • edited Loading

amahussein left a comment

Choose a reason for hiding this comment

amahussein Apr 15, 2025

Choose a reason for hiding this comment

sayedbilalbari Apr 15, 2025 • edited Loading

Choose a reason for hiding this comment

amahussein Apr 16, 2025

Choose a reason for hiding this comment

sayedbilalbari Apr 16, 2025 • edited Loading

Choose a reason for hiding this comment

amahussein commented Apr 15, 2025 • edited Loading

sayedbilalbari commented Apr 15, 2025

sayedbilalbari commented Apr 15, 2025

amahussein left a comment

Choose a reason for hiding this comment

amahussein Apr 16, 2025

Choose a reason for hiding this comment

amahussein Apr 16, 2025

Choose a reason for hiding this comment

sayedbilalbari Apr 18, 2025

Choose a reason for hiding this comment

amahussein commented Apr 24, 2025 • edited Loading

sayedbilalbari commented Apr 28, 2025

amahussein left a comment

Choose a reason for hiding this comment

sayedbilalbari commented Apr 30, 2025

parthosa left a comment

Choose a reason for hiding this comment

amahussein left a comment

Choose a reason for hiding this comment

sayedbilalbari commented May 6, 2025

sayedbilalbari commented Apr 14, 2025 •

edited

Loading

sayedbilalbari Apr 15, 2025 •

edited

Loading

sayedbilalbari Apr 16, 2025 •

edited

Loading

amahussein commented Apr 15, 2025 •

edited

Loading

amahussein commented Apr 24, 2025 •

edited

Loading