Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Include dependency of common jar in tools jar #5270

Merged
merged 11 commits into from
Apr 20, 2022
Merged

Include dependency of common jar in tools jar #5270

merged 11 commits into from
Apr 20, 2022

Conversation

nartal1
Copy link
Collaborator

@nartal1 nartal1 commented Apr 18, 2022

This fixes #5238 .

This PR reverts #5239 where we copied ThreadFactory class in the tools module and removed the dependency on common jar.
In this PR, we added back the dependency on common jar, included only the classes needed from common jar via shade plugin and are deploying the dependency-reduced-pom.xml.
Verified by locally deploying the jar and running mvn dependency:get

@nartal1
Revert "Fix tools depending on the common jar (NVIDIA#5239)"
d6a8ad9 
This reverts commit cec1b7d.
@nartal1
generate reduced pom
2b0034a 
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
@nartal1
include only required classes
44591cf 
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
@nartal1
include ThreadyFactoryBuilder.class
ad28306 
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
@nartal1 nartal1 added the tools label Apr 18, 2022
@nartal1 nartal1 added this to the Apr 18 - Apr 29 milestone Apr 18, 2022
@nartal1 nartal1 self-assigned this Apr 18, 2022
@tgravescs
Copy link
Collaborator

build

tgravescs
tgravescs reviewed Apr 18, 2022
View reviewed changes
tools/pom.xml Outdated Show resolved Hide resolved
@nartal1
Copy link
Collaborator Author

nartal1 commented Apr 19, 2022

build

1 similar comment
@tgravescs
Copy link
Collaborator

build

jlowe
jlowe reviewed Apr 19, 2022
View reviewed changes
tools/pom.xml Outdated Show resolved Hide resolved
@nartal1
add minimizeJar to include classes
ec1a54a 
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
@nartal1
Copy link
Collaborator Author

nartal1 commented Apr 19, 2022

build

jlowe
jlowe previously approved these changes Apr 19, 2022
View reviewed changes
tgravescs
tgravescs previously approved these changes Apr 19, 2022
View reviewed changes
@nartal1
update spark-nightly-build.sh to to deploy dependency reduced pom
fa313f1 
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
@nartal1 nartal1 dismissed stale reviews from tgravescs and jlowe via fa313f1 April 20, 2022 01:26
@nartal1
Copy link
Collaborator Author

nartal1 commented Apr 20, 2022

build

@nartal1
update nightly script
f8c570d 
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
@nartal1
Copy link
Collaborator Author

nartal1 commented Apr 20, 2022

build

NvTimLiu
NvTimLiu previously approved these changes Apr 20, 2022
View reviewed changes
Copy link
Collaborator

@NvTimLiu NvTimLiu left a comment

Choose a reason for hiding this comment

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

LGTM, +1

@NvTimLiu
Copy link
Collaborator

NvTimLiu commented Apr 20, 2022 •
edited
Loading

Dependency tree check PASS

mvn dependency:tree -Dincludes=com.nvidia -s spark-rapids/jenkins/settings.xml -Dpom=spark-rapids/tools/dependency-reduced-pom.xml

[INFO] --- maven-dependency-plugin:2.8:tree (default-cli) @ rapids-4-spark-tools_2.12 ---

[INFO] com.nvidia:rapids-4-spark-tools_2.12:jar:22.06.0-SNAPSHOT

[INFO] Scanning for projects...
[WARNING]
[WARNING] Some problems were encountered while building the effective model for com.nvidia:rapids-4-spark-tools_2.12:jar:22.06.0-SNAPSHOT
[WARNING] 'parent.relativePath' of POM com.nvidia:rapids-4-spark-tools_2.12:22.06.0-SNAPSHOT (/pom.xml) points at com.nvidia:rapids-4-spark-tools_2.12 instead of com.nvidia:rapids-4-spark-parent, please verify your project structure @ line 3, column 11
[WARNING]
[WARNING] It is highly recommended to fix these problems because they threaten the stability of your build.
[WARNING]
[WARNING] For this reason, future Maven versions might no longer support building such malformed projects.
[WARNING]
[INFO]
[INFO] ----------------< com.nvidia:rapids-4-spark-tools_2.12 >----------------
[INFO] Building RAPIDS Accelerator for Apache Spark tools 22.06.0-SNAPSHOT
[INFO] --------------------------------[ jar ]---------------------------------
[INFO]
[INFO] --- maven-dependency-plugin:2.8:tree (default-cli) @ rapids-4-spark-tools_2.12 ---
[INFO] com.nvidia:rapids-4-spark-tools_2.12:jar:22.06.0-SNAPSHOT
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  1.185 s
[INFO] Finished at: 2022-04-20T12:26:15Z
[INFO] ------------------------------------------------------------------------
root@d03af8a1b975:/#

jlowe
jlowe previously approved these changes Apr 20, 2022
View reviewed changes
@nartal1 nartal1 dismissed stale reviews from jlowe and NvTimLiu via 990897c April 20, 2022 19:01
@nartal1
Copy link
Collaborator Author

nartal1 commented Apr 20, 2022

build

@nartal1
Copy link
Collaborator Author

nartal1 commented Apr 20, 2022

Thanks @jlowe , @NvTimLiu and @tgravescs for the review. Merging this PR.

@nartal1 nartal1 merged commit 55177e4 into NVIDIA:branch-22.06 Apr 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tgravescs tgravescs tgravescs approved these changes

@jlowe jlowe jlowe left review comments

@NvTimLiu NvTimLiu NvTimLiu left review comments

@revans2 revans2 Awaiting requested review from revans2 revans2 is a code owner

@GaryShen2008 GaryShen2008 Awaiting requested review from GaryShen2008 GaryShen2008 is a code owner

Assignees

@nartal1 nartal1

Labels
tools
Projects
None yet
Milestone
Apr 18 - Apr 29
Development

Successfully merging this pull request may close these issues.

Tools jar should include common jar rather then copy ThreadFactoryBuilder
4 participants
@nartal1 @tgravescs @NvTimLiu @jlowe