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

Dist artifact with provided aggregator dependency [databricks] #3930

Merged
merged 7 commits into from
Oct 28, 2021
Merged

Dist artifact with provided aggregator dependency [databricks] #3930

merged 7 commits into from
Oct 28, 2021

Conversation

gerashegalov
Copy link
Collaborator

@gerashegalov gerashegalov commented Oct 26, 2021 •
edited
Loading

Closes #3929

  1. Mark aggregator as a provided dependency in dist/pom.xml
  2. Emulate shade dependency-reduced-pom.xml by removing the dependencies section from all profiles.

Signed-off-by: Gera Shegalov gera@apache.org

@gerashegalov
Mark aggregator as provided
f277d5e 
Signed-off-by: Gera Shegalov <gera@apache.org>
@gerashegalov
Manufacture dependency-reduced-pom.xml
657aae7 
Signed-off-by: Gera Shegalov <gera@apache.org>
@gerashegalov gerashegalov self-assigned this Oct 26, 2021
@gerashegalov gerashegalov added the bug Something isn't working label Oct 26, 2021
@gerashegalov gerashegalov added this to the Oct 18 - Oct 29 milestone Oct 26, 2021
@gerashegalov gerashegalov added the build Related to CI / CD or cleanly building label Oct 26, 2021
@gerashegalov gerashegalov linked an issue Oct 26, 2021 that may be closed by this pull request
[BUG] published rapids-4-spark dist artifact references aggregator #3929
Closed
@NvTimLiu
Copy link
Collaborator

build

@gerashegalov
Copy link
Collaborator Author

There is still a problem to address. We need to install-file instead of install in dist to be able to reference the generated pom, and disable default-install execution.

@gerashegalov
Build fixes for reduced pom
5ccf62b 
- customize default-install
- move agggregator dependency to tests, since we no longer have
  transitives via dist

Signed-off-by: Gera Shegalov <gera@apache.org>
@gerashegalov gerashegalov changed the title Dist artifact with provided aggregator dependency Dist artifact with provided aggregator dependency [databricks] Oct 27, 2021
@gerashegalov
Copy link
Collaborator Author

build

@gerashegalov
it compile
1b24f2b
@gerashegalov
More dep fixes
ab1848c 
Signed-off-by: Gera Shegalov <gera@apache.org>
@gerashegalov
Copy link
Collaborator Author

build

jlowe
jlowe reviewed Oct 27, 2021
View reviewed changes
dist/scripts/binary-dedupe.sh Show resolved Hide resolved
tests/pom.xml Show resolved Hide resolved
integration_tests/pom.xml Show resolved Hide resolved
dist/pom.xml Show resolved Hide resolved
dist/scripts/binary-dedupe.sh Show resolved Hide resolved
@NvTimLiu
Copy link
Collaborator

@gera Shegalov Verified the rapids-4-spark jar dependency path got PASS with PR3930, while rapids-4-spark-integration-tests_2.12 jar still has the dependency issue.

mvn dependency:get -B -Ddest=snap -DremoteRepositories=https://oss.sonatype.org/content/repositories/staging -DgroupId=com.nvidia -DartifactId=rapids-4-spark-integration-tests_2.12 -Dversion=21.10.0 -s spark-rapids/jenkins/settings.xml -Dpackaging=jar
ERROR]   Path to dependency:
[ERROR]         1) org.apache.maven.plugins:maven-downloader-plugin:jar:1.0
[ERROR]         2) com.nvidia:rapids-4-spark-integration-tests_2.12:jar:21.10.0
[ERROR]         3) com.nvidia:rapids-4-spark-udf-examples_2.12:jar:21.10.0
 
[ERROR]   Path to dependency:
[ERROR]         1) org.apache.maven.plugins:maven-downloader-plugin:jar:1.0
[ERROR]         2) com.nvidia:rapids-4-spark-integration-tests_2.12:jar:21.10.0
[ERROR]         3) com.nvidia:rapids-4-spark_2.12:jar:21.10.0
[ERROR]         4) com.nvidia:rapids-4-spark-aggregator_2.12:jar:21.10.0
 
[ERROR]   Path to dependency:
[ERROR]         1) org.apache.maven.plugins:maven-downloader-plugin:jar:1.0
[ERROR]         2) com.nvidia:rapids-4-spark-integration-tests_2.12:jar:21.10.0
[ERROR]         3) com.nvidia:rapids-4-spark-tests_2.12:test-jar:spark301tests:21.10.0

BWT, we will release below jars:

rapids-4-spark_2.12
rapids-4-spark-tools_2.12
rapids-4-spark-integration-tests_2.12

@gerashegalov
Reviews
adbe599 
- workaround for integration_tests advertising unpublished depd
  -Dtransitive=false
- fix project version warning by adding install to pluginManagement

Signed-off-by: Gera Shegalov <gera@apache.org>
jlowe
jlowe previously approved these changes Oct 27, 2021
View reviewed changes
tgravescs
tgravescs previously approved these changes Oct 27, 2021
View reviewed changes
@jlowe jlowe mentioned this pull request Oct 27, 2021
Closed
@jlowe
Copy link
Member

jlowe commented Oct 27, 2021

Filed #3934 to track the issue with integration-tests dependencies and #3935 to track revisiting the distribution jar dependencies.

@sameerz
Copy link
Collaborator

sameerz commented Oct 27, 2021

build

@gerashegalov
Copy link
Collaborator Author

Looking at the CI

12:49:30  [ERROR] Failed to execute goal org.apache.maven.plugins:maven-install-plugin:3.0.0-M1:install (default-install) on project rapids-4-spark-sql_2.12: NoFileAssignedException: The packaging plugin for this project did not assign a main file to the project but it has attachments. Change packaging to 'pom'. -> [Help 1]

probably should not declare install-plugin in the parent pluginManagement to avoid cascading changes

@gerashegalov gerashegalov dismissed stale reviews from tgravescs and jlowe via 1915f43 October 27, 2021 20:16
@jlowe
Copy link
Member

jlowe commented Oct 27, 2021

build

@gerashegalov
Copy link
Collaborator Author

the offending step passed locally

NvTimLiu
NvTimLiu approved these changes Oct 28, 2021
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.

Run IT with the this PR, got mvn:get PASS, and integration tests ran well.

@NvTimLiu NvTimLiu merged commit 493ca86 into NVIDIA:branch-21.10 Oct 28, 2021
This was referenced Oct 28, 2021
Closed
Closed
@gerashegalov gerashegalov deleted the aggProvided-3929 branch October 28, 2021 04:52
@jlowe jlowe mentioned this pull request Oct 28, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NvTimLiu NvTimLiu NvTimLiu approved these changes

@jlowe jlowe jlowe approved these changes

@tgravescs tgravescs tgravescs left review comments

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

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

Assignees

@gerashegalov gerashegalov

Labels
bug Something isn't working build Related to CI / CD or cleanly building
Projects
None yet
Milestone
Oct 18 - Oct 29
Development

Successfully merging this pull request may close these issues.

[BUG] published rapids-4-spark dist artifact references aggregator
5 participants
@gerashegalov @NvTimLiu @jlowe @sameerz @tgravescs