Skip to content

Update package build and deploy - #4289

Merged
valeriupredoi merged 9 commits into
mainfrom
update-build-and-deploy
Dec 18, 2025
Merged

Update package build and deploy#4289
valeriupredoi merged 9 commits into
mainfrom
update-build-and-deploy

Conversation

@bouweandela

@bouweandela bouweandela commented Dec 17, 2025

Copy link
Copy Markdown
Member

Description


Before you get started

Checklist

It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.


To help with the number of pull requests:

@bouweandela

Copy link
Copy Markdown
Member Author

Upload to testpypi is failing because we use a local version (the version ends with + followed by g and the first digits of the commit hash) and that is not allowed. Apart from that, this seems to work fine. We could disable the local version, but I kind of like it because it allows distinguishing between development versions from different branches. Maybe we could just skip uploading to testpypi, as we do at the moment. Opinions @valeriupredoi?

@valeriupredoi

Copy link
Copy Markdown
Contributor

Upload to testpypi is failing because we use a local version (the version ends with + followed by g and the first digits of the commit hash) and that is not allowed. Apart from that, this seems to work fine. We could disable the local version, but I kind of like it because it allows distinguishing between development versions from different branches. Maybe we could just skip uploading to testpypi, as we do at the moment. Opinions @valeriupredoi?

it's the dev53+gd85af0253 bit that PyPI doesn't like - no matter it be Test PyPI or the main PyPI - but why are you trying to upload to Test PyPI anyway, if the package builds well - and it does, then upload will work when it's time to release, unless, of course, there's something funky going on with the version. If you really want to test with Test PyPI, then chuck all this here on the v2.13 branch, that'll have a clean version 🍺

@valeriupredoi valeriupredoi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks good, thanks, bud, reminds me I need to do the setuptools pins in a few places myself 🍺

EDIT: I see that now we have Publish: Test PyPI and Publish: main PyPI as two separate jobs - I'd deffo remove the Test job (comment it out so we can reactivate it if need be), it just adds extra guff since it will always be skipped

Comment thread .github/workflows/pypi-build-and-deploy.yml Outdated
Comment thread .github/workflows/pypi-build-and-deploy.yml Outdated
Comment thread .github/workflows/pypi-build-and-deploy.yml Outdated
Co-authored-by: Valeriu Predoi <valeriu.predoi@gmail.com>
@bouweandela

Copy link
Copy Markdown
Member Author

why are you trying to upload to Test PyPI

Because it looks like the packaging guide recommends it. I guess you could catch issues with forgetting to include certain files in the package if you would upload it to test pypi, install from there, and then run the tests, but since we haven't encountered any problems with this so far we may as well skip it.

@valeriupredoi

Copy link
Copy Markdown
Contributor

why are you trying to upload to Test PyPI

Because it looks like the packaging guide recommends it. I guess you could catch issues with forgetting to include certain files in the package if you would upload it to test pypi, install from there, and then run the tests, but since we haven't encountered any problems with this so far we may as well skip it.

yeah that's good practice for newbies, we're all seasoned packagers here, bud. Cheers! Let me approve 🍺

@bouweandela

Copy link
Copy Markdown
Member Author

I also enabled manual approval for the package upload step, as that is recommended for security reasons in https://packaging.python.org/en/latest/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows/#configuring-trusted-publishing

# Use this for testing the package build.
branches:
- main
- v[0-9]+.[0-9]+.x # matches release branches like v2.12.x

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what's this for? That'll run on our mega latest v2.9.x from three years ago 😁
On a serious note, where's that manual approval enabled/configured, in the PyPI project settings?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

what's this for?
To test that the package build works on the release branch.

That'll run on our mega latest v2.9.x from three years ago

Yes, but only if you would push a commit to it.

On a serious note, where's that manual approval enabled/configured, in the PyPI project settings?

https://github.com/ESMValGroup/ESMValTool/settings/environments/8132850576/edit

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ah thanks, I also reckon that's a good idea 🍺

@bouweandela
bouweandela marked this pull request as ready for review December 17, 2025 19:44
@valeriupredoi
valeriupredoi merged commit b515045 into main Dec 18, 2025
8 checks passed
@valeriupredoi
valeriupredoi deleted the update-build-and-deploy branch December 18, 2025 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants