remove git clones and telemetry for package-publishing jobs, use full semver tags for third-party actions - #585
Conversation
jameslamb
added
improvement
jameslamb
changed the title
|
This is ready for review, but shouldn't be merged until we've done the testing in rapidsai/rmm#2458 |
gforsyth
left a comment
gforsyth
left a comment
There was a problem hiding this comment.
I'm all for removing the unnecessary checkouts and adding the full tags for the rest.
As for removing telemetry, I think I'm a +0 -- I haven't seen that particular step fail very often, but it's a job that only runs on main and release/ branches.
If we do have those upload jobs instrumented, I'm trying to think of what we might catch.
If they start taking longer, could mean:
- Artifacts are larger, but we'd catch that anyway from
pydistcheck - Network is having issues (but we'd see that everywhere)
Yeah, I'm good with this. Thanks, James!
I believe that we don't need a source checkout for the RAPIDS CI jobs that publish conda packages and wheels. rapidsai/shared-workflows#585 is proposing removing checkouts and telemetry from those workflows, to save some time and improve CI stability. Proposing that we merge a PR here in `rmm` using those new workflows to test. Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Bradley Dice (https://github.com/bdice) URL: #2458
|
Thanks for the thorough review @gforsyth
We only checks wheels with And technically we only check wheel files with that tool, so that wouldn't be enough to catch that we'd accidentally included other, unexpected files in whatever
I think your summary is right. But I looked around today and I think we might not actually be capturing telemetry data for |
| sha: ${{ inputs.sha }} | ||
| # On 'push' or 'tag', 'inputs.sha' is null. | ||
| # Fall back to 'github.sha' to avoid needing a repo checkout. | ||
| sha: ${{ inputs.sha || github.sha }} |
There was a problem hiding this comment.
The first build of that test RMM PR failed like this:
RAPIDS_REPOSITORY=rapidsai/rmm
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
Error: Process completed with exit code 128.
If the sha input to rapidsai/shared-actions/rapids-github-info is null or an empty string, it falls back to running git rev-parse HEAD.
It'll be null when this workflow is triggered by push (merge to main) or tag.
In those cases, I think it's safe and appropriate to fall back to github.sha if inputs.sha is not provided. Based on my read of https://docs.github.com/en/actions/reference/workflows-and-actions/contexts, that should be the value we want in push or tag contexts.
I restarted rmm CI, let's see what it says: https://github.com/rapidsai/rmm/actions/runs/28388699537
There was a problem hiding this comment.
That worked!
And looks to me like the correct SHA and versions were chosen
https://github.com/rapidsai/rmm/commits/main/
conda:
wheels:
https://github.com/rapidsai/rmm/actions/runs/28388699537
gforsyth
left a comment
gforsyth
left a comment
There was a problem hiding this comment.
Agree with your additional analysis and think this is good to go!
|
/merge |
Contributes to #505 As described there, it's not uncommon for pulling the `rapidsai/ci-wheel` images to take 3-4 minutes. This is extra painful for jobs like `wheels-publish`, which just need lightweight publishing tools and not CUDA libraries, compiler toolchain, etc. This PR proposes working around that by doing the following: * using a small `python:3.14-slim` image instead * installing just the small set of necessary tools at runtime of the `wheels-publish` job ## Notes for Reviewers ### Benefits * faster `conda-uploads-packages` and `wheels-publish`, which means reduced end-to-end time for RAPIDS nightly pipeline (see "how I tested this") * would allow us to stop installing `anaconda-client` and its dependencies in the `rapidsai/ci-conda` and `rapidsai/ci-wheel` images at https://github.com/rapidsai/ci-imgs - _that project has a lot of dependencies, so that might have a small but notable improvement in image size, build time, and pull time_ - _that'd affect all wheel builds and conda building + testing jobs_ ### Costs / Risks * adds more network calls and package installs at runtime of `wheels-publish`, which might lead to more transient failures from network issues * adds complexity to the workflow code ### Why not pre-build a `wheels-publish` image? That WOULD make this even faster and avoid all those package installs at runtime. But new images need to go through a compliance/legal approval process that's a bit heavier than this use case justifies, in my opinion. In my testing, installing tools took 15-20 seconds, so that's the most we'd save by having a pre-built image with everything installed. ### How I tested this `rmm`'s `main` branch was already pointed at this branch for package-uploading jobs from #585. Put up changes in a `gha-tools` branch (rapidsai/gha-tools#265) to force-overwrite existing packages, so the upload time to anaconda.org is included in the timings. Clicked "re-run all jobs" on `rmm`'s most recent run on `main`. | workflow | previous `main` (as of #585) | this PR | |----------------------------------:|:-----------------------------------------:|:------------------:| | upload-conda | 1m32s | **1m15s** | | wheels-publish-cpp | 2m51s | **0m37s** | | wheels-publish-python | 3m3s | **0m41s** | *builds from `rmm`: ([previous main](https://github.com/rapidsai/rmm/actions/runs/28388699537/attempts/2) | [this PR](https://github.com/rapidsai/rmm/actions/runs/28388699537/job/84139196301))* Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Bradley Dice (https://github.com/bdice) URL: #586
2 participants
Contributes to #505
conda-upload-packagesandwheels-publish)And other tiny things I noticed while doing this:
setuptools-scm(RAPIDS does not use this any more)#v7to#v7.0.0in comments onactions/checkout(improves clarity of auto-generated PRs to update that)pre-commithooksNotes for Reviewers
Is this safe?
I think so.
conda-upload-packagesandwheels-publishdon't depend on anything in the calling repo's source or git history.The
rapids-release-buildcheck here sort of depends on git tags:shared-workflows/.github/workflows/conda-upload-packages.yaml
Lines 110 to 112 in ad100ac
But doesn't get it from a local checkout... it reads the GitHub-specific environment variable
GITHUB_REF: https://github.com/rapidsai/gha-tools/blob/d85a30d7a8930b079ef681a086e8f3646120c27a/tools/rapids-is-release-build#L11Impact
Reduces runtime of these jobs a tiny bit, and more importantly removes some network requests (a common source of CI instability).
The impact on runtime will be larger for repos where
git cloneis more expensive. For example, on a recentcudfbuild onmain, checking out the code took 14 seconds and telemetry-related things added around 4 seconds.https://github.com/rapidsai/cudf/actions/runs/28378826926/job/84082197842
How I tested this
Proposing testing this here: rapidsai/rmm#2458