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.

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

add workflow to upload conda packages to legate channel #173

Merged
merged 11 commits into from
Oct 30, 2024

Conversation

jameslamb
Copy link
Member

@jameslamb jameslamb commented Oct 28, 2024

Contributes to #101
Contributes to #115

Follow-up to #171 (comment)

Notes for Reviewers

I've left comments inline describing specific changes in more detail.

@jameslamb jameslamb added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels Oct 28, 2024
@jameslamb
Copy link
Member Author

🎉 🎉 🎉 just pushed conda packages to the experimental label on the legate channel!

image

https://anaconda.org/legate/legate-boost/files

This will now work (try it for yourself)

conda create --name test-legate-boost \
  --dry-run \
  --override-channels \
  -c legate/label/experimental \
  -c legate \
  -c conda-forge \
  -c nvidia \
    legate-boost=24.09.*  \
    python=3.11

This at least shows that as of this branch and moment in time, we have all the pieces in place to start publishing conda packages from this repo to the legate channel. I'll get this PR updated and ready to review 😁

needs:
- conda-python-build
uses: ./.github/workflows/conda-upload-packages.yaml
secrets: inherit
Copy link
Member Author

Choose a reason for hiding this comment

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

Adding this here, with these triggers:

on:
# run on pushes to certain branches
push:
branches:
- "main"
- "release/v[0-9][0-9].[0-9][0-9].[0-9][0-9]"
# run on pushes of new release tags
tags:
- v[0-9][0-9].[0-9][0-9].[0-9][0-9]
# run by clicking buttons in the GitHub Actions UI
workflow_dispatch:

Means that new legate-boost packages will automatically be published to the legate channel in the following situations.

Scenario 1: New packages pushed to the experimental label, with version like {YY}.{MM}.{patch}.dev{n}

  • new commit pushed to main
  • new commit pushed to any branch named release/v{YY}.{MM}.{patch}
  • new tag pushed which does not exactly match the pattern v[0-9][0-9].[0-9][0-9].[0-9][0-9]
  • someone manually clicked the "run workflow" button in GitHub Actions UI

Scenario 2: New packages pushed to the main label, with version like {YY}.{MM}.{patch} (i.e. a stable release)

  • new tag pushed exactly matching the pattern v[0-9][0-9].[0-9][0-9].[0-9][0-9]

Copy link
Contributor

Choose a reason for hiding this comment

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

This sounds reasonable to me. The only downside would be that release branch nightlies are probably not used much and use disk-space/build time.

Copy link
Member Author

Choose a reason for hiding this comment

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

The only downside would be that release branch nightlies are probably not used much and use disk-space/build time.

The release/ branches should only be used for hotfixes, in the way I've proposed setting this up:

### Hotfixes

I'm thinking of the case where @danielfrg or someone reports a bug, and we merge a fix and want to say "ok we have a release candidate of like 24.09.01.dev3, can you test it?"

It'd be useful to have a nightly package for that testing, I think, so we can build confidence in that fix before officially releasing a new version and so the tester doesn't have to build from source or download a GitHub Actions artifact.

I don't know if we really need nightlies for all tags

Here, I'm thinking about the specific case where, say, we just cut stable release 24.09.00 and are ready to begin work on 24.12.00.dev{n}. Was thinking that in that case, we want a first build to run when development starts, so you immediately have nightlies with that version (and the corresponding versions of cunumeric / legate) to use.

See #171 (comment)

@@ -23,8 +23,6 @@ jobs:
# group together all jobs that must pass for a PR to be merged
# (for use by branch protections)
pr-builder:
# skip on builds from merges to main
if: github.ref != 'refs/heads/main'
Copy link
Member Author

Choose a reason for hiding this comment

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

Now that this workflow file is only triggered on pull requests, this condition is unnecessary.

- "=24.09.*,>=0.0.0.dev0"
# .dev319: https://github.com/nv-legate/legate.core.internal/pull/1401
# .dev329: https://github.com/nv-legate/legate.core.internal/issues/1409
- "=24.09.*,>=0.0.0.dev0,!=24.09.00.dev319,!=24.09.00.dev329"
Copy link
Member Author

Choose a reason for hiding this comment

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

I think we should try this approach of !=-ing past nightlies that cause incompatibilities here. This is preferable to == pinning to an older version, because it means that if the bugs we've reported upstream get fixed, then CI will just automatically start pulling in the new nightlies.

And if they aren't, then we'll get feedback from CI of the form "the bug we reported has still not been fixed".

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with the != approach. When we start using a newer version, we can convert the != to > (just to not pile up blocklisted versions)

Copy link
Member Author

Choose a reason for hiding this comment

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

convert the != to >

Yes definitely, agreed!

@jameslamb jameslamb changed the title WIP: [DO NOT MERGE] add workflow to upload conda packages to legate channel add workflow to upload conda packages to legate channel Oct 30, 2024
@jameslamb jameslamb marked this pull request as ready for review October 30, 2024 16:15
Copy link
Contributor

@seberg seberg left a comment

Choose a reason for hiding this comment

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

Thanks @jameslamb! I looked through and the logic seems right to me. I don't know if we really need nightlies for all tags and release branches (rather than just main).
But, there is also no harm, unless disk space is a problem and since versions must match very precisely, it could be useful.

- "=24.09.*,>=0.0.0.dev0"
# .dev319: https://github.com/nv-legate/legate.core.internal/pull/1401
# .dev329: https://github.com/nv-legate/legate.core.internal/issues/1409
- "=24.09.*,>=0.0.0.dev0,!=24.09.00.dev319,!=24.09.00.dev329"
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with the != approach. When we start using a newer version, we can convert the != to > (just to not pile up blocklisted versions)

needs:
- conda-python-build
uses: ./.github/workflows/conda-upload-packages.yaml
secrets: inherit
Copy link
Contributor

Choose a reason for hiding this comment

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

This sounds reasonable to me. The only downside would be that release branch nightlies are probably not used much and use disk-space/build time.

@jameslamb
Copy link
Member Author

I'm going to merge this on the strength of your approval @seberg , to test that it does what we want on merge-to-main, and to move on to the next step in this.

All of these things are easily reversible, so @RAMitchell please do let me know, when you see this, if there are any changes you'd like me to make.

@jameslamb jameslamb merged commit e998041 into rapidsai:main Oct 30, 2024
9 checks passed
@jameslamb jameslamb deleted the package-publishing branch October 30, 2024 18:49
@jameslamb
Copy link
Member Author

Cool yeah this worked!

The merge triggered this build: https://github.com/rapidsai/legate-boost/actions/runs/11599749514/job/32298585013

That published 6 new packages to the legate channel (3 Python versions x [cpu, gpu]), with version 24.09.00.dev1

image

https://anaconda.org/legate/legate-boost/files

@RAMitchell
Copy link
Contributor

Looks great thank you @jameslamb!

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.

3 participants