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

[jvm-packages] Clean up CI #10982

Merged
merged 12 commits into from
Nov 14, 2024
Merged

[jvm-packages] Clean up CI #10982

merged 12 commits into from
Nov 14, 2024

Conversation

hcho3
Copy link
Collaborator

@hcho3 hcho3 commented Nov 5, 2024 •
edited
Loading

Closes #10879
Closes #7262

  • Properly handle shaded JARs
  • Remove xgboost4j-tester; fold integration tests as part of the test suite, so that mvn test would suffice.
  • Add test coverage for all demos. Update the demos so that they can run without failing.
  • Remove unused Maven profiles
  • Update the CI pipelines to deploy the shaded JARs (xgboost4j-spark / xgboost4j-spark-gpu)

Note. I will submit a follow-up PR to update dev/prepare_jvm_release.py.

New workflow:

  1. mvn deploy -Pdefault,release-to-s3, to deploy all CPU packages (xgboost4j, xgboost4j-example, xgboost4j-spark, xgboost4j-flink).
  2. mvn clean to clean all artifacts
  3. mvn install -Pgpu, to build xgboost4j and xgboost4j-spark-gpu with CUDA enabled and to install them locally.
  4. mvn deploy -Pgpu,release-to-s3 -pl xgboost4j-spark-gpu to deploy xgboost4j-spark-gpu only.

@hcho3
[WIP] Clean up CI for JVM packages
9569a7b 
- Properly handle shaded JARs
- Remove xgboost4j-tester
- Remove unused Maven profiles
@hcho3 hcho3 marked this pull request as draft November 5, 2024 02:47
@hcho3 hcho3 mentioned this pull request Nov 5, 2024
Closed
@hcho3 hcho3 marked this pull request as ready for review November 8, 2024 04:14
@hcho3 hcho3 changed the title [WIP] Clean up CI for JVM packages Clean up CI for JVM packages Nov 8, 2024
@hcho3 hcho3 changed the title Clean up CI for JVM packages [jvm-packages] Clean up CI Nov 8, 2024
@hcho3
Copy link
Collaborator Author

hcho3 commented Nov 8, 2024 •
edited
Loading

@wbo4958 The PR is now ready for review.

See the full build log at https://buildkite.com/organizations/xgboost/pipelines/xgboost-ci/builds/7247/jobs/01930a4e-98e2-4dad-bc10-ab2fdd58f376/log

@hcho3 hcho3 requested a review from trivialfis November 8, 2024 07:11
@trivialfis
Copy link
Member

Remove xgboost4j-tester

Is there an alternative? We have no test for xgboost jvm demos.

@hcho3
Copy link
Collaborator Author

hcho3 commented Nov 8, 2024

I'll bring back the integration tests.

@hcho3 hcho3 marked this pull request as draft November 11, 2024 05:09
@hcho3
Copy link
Collaborator Author

hcho3 commented Nov 11, 2024

Marking as draft until I bring back the integration tests.

@hcho3 hcho3 marked this pull request as ready for review November 12, 2024 03:16
@hcho3
Copy link
Collaborator Author

hcho3 commented Nov 12, 2024

@wbo4958 I moved the integration tests so that mvn test -pl xgboost4j-example will run them. I discovered that some demos were left out in the previous version of integration tests. Now all demos have test coverage.

@trivialfis
Copy link
Member

I assume we can close this one as well #7262 ?

trivialfis
trivialfis approved these changes Nov 12, 2024
View reviewed changes
Copy link
Member

@trivialfis trivialfis left a comment

Choose a reason for hiding this comment

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

Huge thanks for the cleanup and fixes! I will leave it to @wbo4958 for a detailed review for the pom files.

wbo4958
wbo4958 reviewed Nov 14, 2024
View reviewed changes
jvm-packages/pom.xml
@@ -19,8 +19,16 @@
</licenses>
<developers>
<developer>
<name>CodingCat</name>
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably leave CodingCat here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure.

wbo4958
wbo4958 approved these changes Nov 14, 2024
View reviewed changes
Copy link
Contributor

@wbo4958 wbo4958 left a comment

Choose a reason for hiding this comment

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

LGTM

@hcho3 hcho3 merged commit b835917 into dmlc:master Nov 14, 2024
25 of 30 checks passed
@hcho3 hcho3 deleted the fix_jvm_deploy branch November 14, 2024 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trivialfis trivialfis trivialfis approved these changes

@wbo4958 wbo4958 wbo4958 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@hcho3 @trivialfis @wbo4958