google-research / torchsde

Differentiable SDE solvers with GPU support and efficient sensitivity analysis.
Apache License 2.0
1.51k stars 194 forks source link

Extend CI workflow to include PyPI dist publishing #100

Closed Zymrael closed 2 years ago

Zymrael commented 2 years ago

Minor PR, title says it all.

Publishing is triggered only by release events. To work, the addition requires setup of a PyPI password in repo secrets.

Addresses this.

google-cla[bot] commented 2 years ago

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

Zymrael commented 2 years ago

@googlebot I signed it!

patrick-kidger commented 2 years ago

So I note that this will trigger on quite a lot of things, including a cron schedule. It would be preferred to have it trigger only when we merge a pull request into master; moreover it'd need to check that the version number has been incremented, to prevent PyPI failures.

That said, I'd be keen to have this happen, as it's realistic that waiting for me or @lxuechen to push a button will be a bottleneck in the process... :D If you can rewrite it as above I'd be happy to have this in.

Zymrael commented 2 years ago

It sure will trigger the entire workflow but it will skip the publishing step. That last step (atm) is only triggered via release events. There seems to be a check for version increases in the current workflow put into place by @lxuechen.

Of course if you'd prefer we can split the current workflow into two. Remove the current release check, and instead have a secondary publishing workflow that only triggers on PR merges if the existing version check passes.

Should be quick but since there are roughly infinity ways of doing this I'll await for you to state your preference first :)

patrick-kidger commented 2 years ago

Ah yes, I see the if statement now. I think the if statement will never trigger though, because the on: section above doesn't include release events?

So I think creating a separate workflow that does this, and triggers on release events, would probably be smoothest?

Pinging @lxuechen as the only one who can actually add the necessary secret (the PyPI key) for this.

lxuechen commented 2 years ago

Ah yes, I see the if statement now. I think the if statement will never trigger though, because the on: section above doesn't include release events?

So I think creating a separate workflow that does this, and triggers on release events, would probably be smoothest?

Pinging @lxuechen as the only one who can actually add the necessary secret (the PyPI key) for this.

I agree with Patrick here. What's more, it seems this publishing step will be ran more than once due to the multiple run environments constructed by the build matrix. Splitting the publishing step to a separate workflow is a good idea. Enforcing dependencies among workflows is a little tricky, but can be done with workflow_run.

I have created the necessary secret based on an API token, but might need Patrick to add me to the project on PyPI.

patrick-kidger commented 2 years ago

Right, I think I've done everything that should be necessary to get this working. I've opened three small PRs (#102,#103,#104) to add all the necessary functionality, in place of this one.

Zymrael commented 2 years ago

I will be closing this PR then.