scientific-python / upload-nightly-action

This action is used to upload nightly builds of your package.
https://anaconda.org/scientific-python-nightly-wheels
BSD 3-Clause "New" or "Revised" License
7 stars 10 forks source link

ENH: Use pixi to run action as composite #91

Closed matthewfeickert closed 1 month ago

matthewfeickert commented 2 months ago

Resolves #71

Description

[!NOTE] GitHub Actions takes care of cleaning up the environment after each run, so you are not responsible for manually unsetting all environment variables between steps to avoid secret leakage.

bsipocz commented 2 months ago

Nitpicking: @matthewfeickert could we follow a fork-based workflow here in the future, so the main repo won't be peppered with feature branches?

matthewfeickert commented 2 months ago

could we follow a fork-based workflow here in the future, so the main repo won't be peppered with feature branches?

@bsipocz I would, but c.f. https://github.com/scientific-python/upload-nightly-action/pull/81#issuecomment-2151542008 for why if we want CI to run we can't. (I also don't find a transient feature branch terrible, but I fully agree that a fork based workflow is the way to go.)

bsipocz commented 2 months ago

I also don't find a transient feature branch terrible

The issue is that some of the branches are not exactly transient, but are around for a while when part of some experimenting or draft PR.

bsipocz commented 2 months ago

Re CI: I think it's OK to separate the current workflow into two, one that runs all the anaconda upload/cleanup checks, and one that does everything else. Then we can run most of the tests from forks, too, and in practice those would cover most of the PR cases anyway.

bsipocz commented 2 months ago

(however, that said, dependabot PRs, from local feature branches are also failing with TOKEN issues: https://github.com/scientific-python/upload-nightly-action/pull/86)

matthewfeickert commented 2 months ago

Re CI: I think it's OK to separate the current workflow into two, one that runs all the anaconda upload/cleanup checks, and one that does everything else. Then we can run most of the tests from forks, too, and in practice those would cover most of the PR cases anyway.

@bsipocz I don't think that will work for the reasons I describe in the two previous responses I've linked, as you'll need secrets to run the upload and a secret to delete the files and neither can be run on a fork safely. But can you make an Issue to describe more what you have in mind here?

Full disclosure, there may be some clever (or obvious :P) way to get around this I've overlooked so far.

tupui commented 2 months ago

Cool 👍 Just a fly by comment. What do you think about uv otherwise? Looks like this is going to be the thing sooner than later.

bsipocz commented 2 months ago

I don't think that will work for the reasons I describe in the two previous responses I've linked, as you'll need secrets to run the upload and a secret to delete the files and neither can be run on a fork safely.

Yes, my point is to separate those that need the secret in a separate workflow/job, so 1) tests can pass on forks with the acknowledgement that the secret-requiring ones are skipped. (most of the times that approach should be good enough)

But also, something is broken with the current setup as e.g. #86 failed even though it run from a feature branch here.

matthewfeickert commented 2 months ago

What do you think about uv otherwise? Looks like this is going to be the thing sooner than later.

@tupui I like uv a lot and I use it on many projects. uv pip compile is excellent and I use it in container runtimes where I don't have control over the Python runtime given the base image environment (this is actually one area/thing that pixi doesn't really allow you to do as it needs to control the entire environment — some caveats there if you export the shell activation though and manually use it.)

That being said, I like pixi more in general, and I think for something like this where you want to have hash level reproducibility of the entire stack and not just of the Python packages then pixi is a clear winner (well really more just "locked conda installs in an isolated/containerized environment" is the winner).

tupui commented 2 months ago

Interesting thanks for the write up!

matthewfeickert commented 2 months ago

@scientific-python/nightly-wheels-developers this is now ready for review. As I had this in draft before to figure things out I've intentionally squashed and rebased this so that it will be easier to follow my actual intended changes from both the GitHub UI and from a local checkout.

@pavelzw @ruben-arts as you've contributed the most to the setup-pixi action if you're able to review this as well that would be very much appreciated.

edit: Also pinging @stefanv too as he's not on the @scientific-python/nightly-wheels-developers list but was kind enough to give an initial review. :)

matthewfeickert commented 1 month ago

Thanks for reviewing for @scientific-python/nightly-wheels-developers, @MridulS! :pray: If we're able to get one more review from another @scientific-python/nightly-wheels-developers person that would be great, but if no one else reviews by Tuesday (2024-10-01) I'll go ahead and squash and merge.

matthewfeickert commented 1 month ago

Thanks @bsipocz!

I'm going to do a relock and it there's any changes to a rebase and push and then squash and merge this. I'll then update the v0.5.0 info in the repo to v0.6.0 and make a new release on GitHub.