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

enable manual trigger for autorelease part 2 #1493 #1496

Merged
merged 2 commits into from
Jan 4, 2024

Conversation

emlys
Copy link
Member

@emlys emlys commented Jan 4, 2024

Description

Fixes #1493, fixes #1494

Unfortunately there's no simple way to test these changes. I did confirm that the trigger works on my fork.

Checklist

  • Updated HISTORY.rst and link to any relevant issue (if these changes are user-facing)
  • Updated the user's guide (if needed)
  • Tested the Workbench UI (if relevant)

@@ -58,8 +70,6 @@ jobs:
# move the artifacts out of the folders
mv artifacts/*/* artifacts
rm -rf artifacts/Wheel*
# verify that all 6 wheels are there
if [ `find artifacts/* | wc -l` -ne 6 ]; then exit 1; fi
Copy link
Member Author

Choose a reason for hiding this comment

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

Without this check, if we change the name of the wheel artifacts, and forget to update it here, then the release will proceed without the necessary wheels. It seems like you can upload files to a release after it's published, so this shouldn't be a critical issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, that sounds good.

Actually it has me thinking that the more dangerous part is the artifacts for PyPI, since we can't modify that release after it's published. Like what if we changed the artifact names from "Wheel for *". We might not have any wheels downloaded, but we might still call twine upload artifacts/natcap.invest* with the source distribution natcap.invest....tgz available.

@emlys emlys self-assigned this Jan 4, 2024
@emlys emlys requested a review from davemfish January 4, 2024 19:22
@emlys emlys marked this pull request as ready for review January 4, 2024 19:22
Copy link
Contributor

@davemfish davemfish left a comment

Choose a reason for hiding this comment

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

This looks good, thanks, @emlys . In the event we accidentally trigger this without doing part 1, things should fail when it tries to find the non-existent Actions run for the tag.

Your comment about the potential for release artifact name-changes made me think about that case for the PyPI artifacts. There might be a fail-case to prevent there. But we don't necessarily need to make any more changes to this workflow right now. What do you think?

@@ -58,8 +70,6 @@ jobs:
# move the artifacts out of the folders
mv artifacts/*/* artifacts
rm -rf artifacts/Wheel*
# verify that all 6 wheels are there
if [ `find artifacts/* | wc -l` -ne 6 ]; then exit 1; fi
Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, that sounds good.

Actually it has me thinking that the more dangerous part is the artifacts for PyPI, since we can't modify that release after it's published. Like what if we changed the artifact names from "Wheel for *". We might not have any wheels downloaded, but we might still call twine upload artifacts/natcap.invest* with the source distribution natcap.invest....tgz available.

@emlys
Copy link
Member Author

emlys commented Jan 4, 2024

@davemfish my understanding is you can upload additional files to a PyPI release too. On testpypi I am able to add another file after the first upload to a particular release.

@davemfish
Copy link
Contributor

@davemfish my understanding is you can upload additional files to a PyPI release too. On testpypi I am able to add another file after the first upload to a particular release.

Ahh okay, great! Thanks for correcting me, I was blindly assuming it was not possible because of the general strictness of things on PyPI.

@davemfish davemfish merged commit 23fd4a8 into natcap:main Jan 4, 2024
25 checks passed
@emlys emlys deleted the feature/1493 branch October 3, 2024 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants