Skip to content

Conversation

@oarbusi
Copy link
Collaborator

@oarbusi oarbusi commented Jun 3, 2025

Proposed changes

Signs artifacts released of PyPi

Link to any related issue(s): CLOUDP-321620

Type of change:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as
    expected)
  • This change requires a documentation update

Required Checklist:

  • I have signed the MongoDB CLA
  • I have added tests that prove my fix is effective or that my feature works
  • I have added any necessary documentation (if appropriate)
  • I have run make fmt and formatted my code
  • I have tested the CDK constructor in a CFN stack. See TESTING.md
  • If changes include removal or addition of 3rd party GitHub actions, I updated our internal document. Reach out to the APIx Integration slack channel to get access to the internal document.

Further comments

Copilot AI review requested due to automatic review settings June 3, 2025 07:51
@oarbusi oarbusi requested a review from a team as a code owner June 3, 2025 07:51
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances the PyPI release workflow by adding GPG signing of distribution artifacts and switching to Twine for uploads.

  • Imports the GPG key into the runner
  • Signs wheel and tarball artifacts with GPG
  • Replaces the publib-pypi step with twine upload

env:
APIX_BOT_GPG_PASSPHRASE: ${{ secrets.APIX_BOT_GPG_PASSPHRASE }}

- name: Upload to PyPI
Copy link

Copilot AI Jun 3, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider using Twine's built-in --sign flag to both sign and upload artifacts in one command (e.g., twine upload --sign dist/*), which can simplify the workflow by removing the separate GPG step.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

@EspenAlbert EspenAlbert left a comment

Choose a reason for hiding this comment

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

Have we considered using https://github.com/pypa/gh-action-pypi-publish directly?
Not sure if we have access to pypi, but this way we wouldn't even need TWINE_USERNAME or TWINE_PASSWORD, with oidc the github action can use its own identity.

@oarbusi
Copy link
Collaborator Author

oarbusi commented Jun 3, 2025

@EspenAlbert No I haven't considered using https://github.com/pypa/gh-action-pypi-publish. As far as I know, we don't have access to PyPi. I have tried to keep changes to a minimum. Since https://github.com/cdklabs/publib was using twine, I think there shouldn't be an issue to keep this way of publishing for now.

TWINE_USERNAME: ${{ secrets.TWINE_USERNAME }}
TWINE_PASSWORD: ${{ secrets.TWINE_PASSWORD }}
run: npx -p publib@latest publib-pypi
run: twine upload dist/*
Copy link
Member

Choose a reason for hiding this comment

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

have you been able to test it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not able to test if we don't want to release 😆 , but I tried to keep changes to a minimum (do only what publib was doing)

Copy link
Member

Choose a reason for hiding this comment

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

i understand, but just to make aware that we might have some issues in the next CDK release

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed, we'll have to be careful on the next release

with:
gpg_private_key: ${{ secrets.APIX_BOT_GPG_PRIVATE_KEY }}
passphrase: ${{ secrets.APIX_BOT_GPG_PASSPHRASE }}
- name: GPG sign PyPI distributions
Copy link
Member

Choose a reason for hiding this comment

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

have you been able to test it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This command I have tested, yes

@oarbusi oarbusi merged commit d328458 into main Jun 4, 2025
13 checks passed
@oarbusi oarbusi deleted the CLOUDP-321620 branch June 4, 2025 10:07
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