Move AssertUtils to the api submodule#13654
Conversation
Signed-off-by: Alessandro Bellina <abellina@nvidia.com>
There was a problem hiding this comment.
Pull Request Overview
This PR addresses an issue where AssertUtils was being shimmed when running in nightly builds, making it inaccessible from the integration_tests submodule. The fix moves AssertUtils to the api submodule (which is not shimmed) and adds the necessary dependency.
- Moved
AssertUtilsclass from a shimmed location to theapisubmodule - Added
rapids-4-spark-sql-plugin-apidependency tointegration_testssubmodule
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR relocates AssertUtils.scala from the sql-plugin module to the sql-plugin-api module to resolve a nightly build failure. The issue arose because nightly builds use "parallel-world" shimming, which rewrites class package names to version-specific namespaces (e.g., sparkver/com/nvidia/...). When AssertUtils lived in sql-plugin, it was shimmed and relocated, breaking imports in integration_tests that expected it at com.nvidia.spark.rapids.AssertUtils. The sql-plugin-api module is explicitly excluded from shimming, so moving the class there keeps it in a stable namespace across all build types (local, pre-merge, and nightly). A corresponding dependency on rapids-4-spark-sql-plugin-api was added to integration_tests/pom.xml with provided scope, since the api module is already on the runtime classpath.
Confidence score: 5/5
- This PR is safe to merge with minimal risk as it addresses a specific build infrastructure issue without altering business logic.
- Score reflects a straightforward refactoring that moves a single utility class to a more appropriate module to avoid shimming side-effects; no code behavior changes.
- No files require special attention, though verify that nightly builds succeed and
integration_testscan resolvecom.nvidia.spark.rapids.AssertUtilswithout errors.
2 files reviewed, no comments
|
build |
|
also requires an update to scala2.13/pom file |
|
build |
|
build |
Fixes #13647
Description
Ok, so I believe the integration tests locally and in pre-merge are running against a specific non-parallel-world jar, so all classes are in the expected namespaces (not scribbled away over at sparkver/com/nvidia..), but the nightly doesn't seem to be doing that.
I had added a class that I need to be able to read from the
integration_testssubmodule. And that seems to be getting shimmed.Here are my changes to help fix:
integration_testto get the api submodule.I am having trouble testing locally (network). I'd like @gerashegalov's opinion on this patch, as he knows the build system better than anyone.
Checklists
(Please explain in the PR description how the new code paths are tested, such as names of the new/existing tests that cover them.)