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

run wheels-publish jobs in Python 3.11 image #169

Merged
merged 4 commits into from
Mar 4, 2024
Merged

Conversation

jameslamb
Copy link
Member

Contributes to rapidsai/build-planning#7.

Split off from #166.

This proposes upgrading the image used in the wheels-publish workflow to a newer Python version (3.10 -> 3.11).

Benefits of this change

Delays similar updates further into the future (e.g. aws and anaconda-client will drop Python 3.10 support before they drop Python 3.11 support).

How I tested this

This workflow just runs this script: https://github.com/rapidsai/gha-tools/blob/main/tools/rapids-wheels-anaconda.

So just checked that the things that script needs are in the image.

docker run --rm -it rapidsai/ci-wheel:cuda12.0.1-centos7-py3.11 bash

which rapids-wheels-anaconda
# /usr/local/bin/rapids-wheels-anaconda

aws --version
# aws-cli/2.15.7 Python/3.11.6 Linux/5.4.0-148-generic docker/x86_64.centos.7 prompt/off

twine --version
# twine version 4.0.2
# (importlib-metadata: 7.0.1, keyring: 24.3.0, pkginfo: 1.9.6, requests: 2.31.0, requests-toolbelt:
# 1.0.0, urllib3: 2.1.0)

anaconda --version
# anaconda Command line client (version 1.12.2)

I think that's probably sufficient. It'd be easy to revert this if it breaks something... less effort and lower-risk than trying to test on another repo where this wheels-publish only runs on merges to main.

# it's simply a launcher for twine
image: "rapidsai/ci-wheel:cuda12.0.1-centos7-py3.10"
# CUDA toolkit version of the container is irrelevant in the publish step.
# This just uploads already-built wheels to remote storage.
Copy link
Member Author

@jameslamb jameslamb Jan 4, 2024

Choose a reason for hiding this comment

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

This workflow no longer uses twine, as of #154.

Proposing changing this comment to something more future-proof.

@jameslamb jameslamb marked this pull request as ready for review January 4, 2024 17:20
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

I'm okay with this change but I would consider delaying the merge this until we begin work (or complete work) on rapidsai/build-planning#3, currently targeting release 24.04. It doesn't benefit us very much to jump ahead with a new Python version until it's widely used across RAPIDS. Moving all of RAPIDS in lockstep is my preference.

@jameslamb
Copy link
Member Author

Ok makes sense, thanks for that! I agree, this should only be merged as part of rapidsai/build-planning#3, in whatever release that targets.

@bdice bdice marked this pull request as draft January 9, 2024 01:26
@bdice
Copy link
Contributor

bdice commented Jan 9, 2024

I converted this to a draft for now. We can reopen once we're moving forward on Python 3.11.

@vyasr
Copy link
Contributor

vyasr commented Jan 11, 2024

The Python version used in this jobs can safely be decoupled from everything else, but conceptually I'm fine with keeping things in sync. No rush here.

@jameslamb jameslamb changed the base branch from branch-24.02 to branch-24.04 January 22, 2024 16:16
@jameslamb jameslamb mentioned this pull request Feb 20, 2024
6 tasks
@jakirkham
Copy link
Member

As we are now adding Python 3.11, marking ready for review

@jakirkham jakirkham marked this pull request as ready for review March 4, 2024 19:16
@bdice
Copy link
Contributor

bdice commented Mar 4, 2024

I created a test PR for cudf here: rapidsai/cudf#15227

If that passes, we can close it and merge this PR.

@bdice
Copy link
Contributor

bdice commented Mar 4, 2024

I realized that the wheels-publish workflow only runs on build.yaml, not pr.yaml. So rapidsai/cudf#15227 isn't a sufficient test. There isn't a good way to test this because it only triggers when PRs are merged (or during nightly tests). I'm going to go ahead and merge it so we can get feedback from nightlies and revert tomorrow if necessary.

@bdice bdice merged commit 84a3009 into branch-24.04 Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants