Skip to content
This repository was archived by the owner on Nov 8, 2024. It is now read-only.

bug: [Consolidate publishing and signing distributions into one step]#61

Merged
leoromanovsky merged 4 commits intomainfrom
fixup
Jul 16, 2024
Merged

bug: [Consolidate publishing and signing distributions into one step]#61
leoromanovsky merged 4 commits intomainfrom
fixup

Conversation

@leoromanovsky
Copy link
Member

@leoromanovsky leoromanovsky commented Jul 15, 2024


labels: mergeable

Fixes: #issue

Motivation and Context

Publish with signing fails because at that step the release has already been made; it attempts to create another one: https://github.com/Eppo-exp/python-sdk/actions/runs/9945729712/job/27474683642

This seems to only happen to me because I'm used to creating a Github release like in the other SDKs. If just pushing to a tag without a release the process likely works.

Description

  • Combine the two publishing steps (publishing and publishing with signing) into one.
  • Add readme for developers.

How has this been tested?

Difficult to test: suggestions welcome but we might need to iterate on it.

@leoromanovsky leoromanovsky marked this pull request as ready for review July 15, 2024 20:12
Copy link

@giorgiomartini0 giorgiomartini0 left a comment

Choose a reason for hiding this comment

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

🙌

Copy link
Contributor

@aarsilv aarsilv left a comment

Choose a reason for hiding this comment

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

Thanks for making this more similar to other SDKs! I like having creating a GitHub release as the common way to publish across SDKs

push:
tags:
- 'v*'
workflow_dispatch:
Copy link
Contributor

Choose a reason for hiding this comment

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

this line is what allows manual publishing; we may want to keep it

Copy link
Member Author

Choose a reason for hiding this comment

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

What is the purpose of manual publishing?

Copy link
Contributor

Choose a reason for hiding this comment

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

Debugging. It would allow us to retry the workflow without making a new release, and while we can retry a failed release action, it can be handy for retrying when the action technically succeeds but didn't do what you want.

Comment on lines -80 to -97
- name: Create GitHub Release
env:
GITHUB_TOKEN: ${{ github.token }}
run: >-
gh release create
'${{ github.ref_name }}'
--repo '${{ github.repository }}'
--notes ""
- name: Upload artifact signatures to GitHub Release
env:
GITHUB_TOKEN: ${{ github.token }}
# Upload to GitHub Release using the `gh` CLI.
# `dist/` contains the built packages, and the
# sigstore-produced signatures and certificates.
run: >-
gh release upload
'${{ github.ref_name }}' dist/**
--repo '${{ github.repository }}'
Copy link
Member Author

Choose a reason for hiding this comment

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

The github release already exists at this point so this is not needed.

Comment on lines +55 to +60
- name: Sign the dists with Sigstore
uses: sigstore/gh-action-sigstore-python@v2.1.1
with:
inputs: >-
./dist/*.tar.gz
./dist/*.whl
Copy link
Member Author

Choose a reason for hiding this comment

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

combine into one publish

Comment on lines -4 to -7
push:
tags:
- 'v*'
workflow_dispatch:
Copy link
Member Author

Choose a reason for hiding this comment

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

conforms to readme

push:
tags:
- 'v*'
workflow_dispatch:
Copy link
Member Author

Choose a reason for hiding this comment

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

What is the purpose of manual publishing?

@leoromanovsky leoromanovsky merged commit ea46eb1 into main Jul 16, 2024
@leoromanovsky leoromanovsky deleted the fixup branch July 16, 2024 02:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants