Skip to content

Adding scripts to download and upload artifacts to the Github Artifact Store - #152

Merged
ajschmidt8 merged 27 commits into
rapidsai:mainfrom
jayavenkatesh19:test-github-artifacts
Mar 14, 2025
Merged

Adding scripts to download and upload artifacts to the Github Artifact Store#152
ajschmidt8 merged 27 commits into
rapidsai:mainfrom
jayavenkatesh19:test-github-artifacts

Conversation

@jayavenkatesh19

Copy link
Copy Markdown
Contributor

Adding new scripts which use the Github Artifact Store to upload and download build artifacts towards https://github.com/rapidsai/build-infra/issues/237. No existing scripts or functionality is altered.

@jayavenkatesh19 jayavenkatesh19 added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels Mar 11, 2025
@jayavenkatesh19 jayavenkatesh19 self-assigned this Mar 11, 2025
@jayavenkatesh19
jayavenkatesh19 requested a review from a team as a code owner March 11, 2025 20:52
@jayavenkatesh19
jayavenkatesh19 requested review from gforsyth and removed request for a team March 11, 2025 20:52
@jameslamb
jameslamb self-requested a review March 11, 2025 22:53

@jameslamb jameslamb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm going to join here as a reviewer alongside @gforsyth since you and I have been talking about this work and since I'd done an earlier version of it.

To start... how did you test this? Could we see some PRs showing how these would be used?

@jayavenkatesh19

jayavenkatesh19 commented Mar 11, 2025

Copy link
Copy Markdown
Contributor Author

Hey @jameslamb, Here are a couple of PRs on RMM and cuDF which use the new gha-tools scripts which are a part of this PR

  1. RMM: Streamlining wheel builds to use fixed location and uploading build artifacts to Github rmm#1810
  2. cuDF: [DO NOT MERGE] Test PR for moving build artifacts to Github Artifact Store NVIDIA/cudf#18090

To test these changes, I used these scripts from my gha-tools fork which is overwriting the existing gha-tools folder present on the build Docker images. This customization is on the test-github-artifacts branch of the shared-workflows repository. You can see those changes here: rapidsai/shared-workflows@branch-25.04...test-github-artifacts.

Let me know if you need any additional details, I'll be happy to discuss them with you!

@gforsyth gforsyth left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for putting this together @VenkateshJaya !

I have some bash simplifications (if such a thing can be said to exist) and a few questions about usage, but I think this tooling will be a big improvement!

Comment thread tools/rapids-download-wheels-from-github Outdated
Comment thread tools/rapids-get-pr-conda-artifact-github Outdated
Comment thread tools/rapids-get-pr-wheel-artifact-github Outdated
Comment thread tools/rapids-github-run-id
Comment thread tools/rapids-github-run-id Outdated
Comment thread tools/rapids-download-from-github
Comment thread tools/rapids-download-conda-from-github Outdated
Comment thread tools/rapids-download-conda-from-github
Comment thread tools/rapids-get-pr-conda-artifact-github Outdated
Comment thread tools/rapids-get-pr-wheel-artifact-github Outdated
Comment thread tools/rapids-get-pr-conda-artifact-github Outdated

@ajschmidt8 ajschmidt8 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good far! Left a few comments, but we're getting closer.

Comment thread tools/rapids-download-conda-from-github Outdated
Comment thread tools/rapids-download-wheels-from-github Outdated
Comment thread tools/rapids-get-pr-conda-artifact-github Outdated

@ajschmidt8 ajschmidt8 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nice! It's coming along.

I tested some of these scripts locally and found one issue. The rapids-download-*-from-github and rapids-get-pr-*-artifact-github scripts fails because rapids-package-name still returns the .tar.gz extension. To fix this, I updated the end of the rapids-package-name script to include the following logic:

diff --git a/tools/rapids-package-name b/tools/rapids-package-name
index eaadc5c..945f2d0 100755
--- a/tools/rapids-package-name
+++ b/tools/rapids-package-name
@@ -76,6 +76,11 @@ if (( append_arch == 1)); then
 fi
 
 # for cpp and python package types, it's a tarball, append .tar.gz and prepend project name
-pkg_name="${repo_name}_${pkg_name}.tar.gz"
+pkg_extension=".tar.gz"
+if [[ "${RAPIDS_NO_PKG_EXTENSION:-}" == "true" ]]; then
+  pkg_extension=""
+fi
+
+pkg_name="${repo_name}_${pkg_name}${pkg_extension}"
 
 echo -n "${pkg_name}"

then I was able to set RAPIDS_NO_PKG_EXTENSION=true in my local environment and everything started working correctly.

Can you make these changes to your scripts? This change will also require that you update the package extension handling in rapidsai/shared-workflows#297.

@ajschmidt8

Copy link
Copy Markdown
Member

in hindsight, I probably should've suggested using basename to make that diff cleaner. but those conditionals and env vars will get removed after this migration is over, so it's a moot point.

@gforsyth gforsyth left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One suggestion and one question, but nothing blocking. This looks really nice @VenkateshJaya !

Comment thread tools/rapids-download-from-github
Comment thread tools/rapids-get-pr-wheel-artifact-github Outdated
jayavenkatesh19 and others added 3 commits March 14, 2025 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improves an existing functionality non-breaking Introduces a non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants