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

Python: publish on version bump only #230

Merged
merged 4 commits into from
Dec 11, 2023
Merged

Conversation

kplattret
Copy link
Member

@kplattret kplattret commented Dec 5, 2023

Since #228, the Python workflow will run for any changes in the Python
code, the shared test resources and the Python workflow itself. This
means that when the library version does not change, the publish step
simply fails, which does not look good.

This updates the Python workflow so that it will only attempt to publish
the library automatically when there is a version bump on main.

@kplattret kplattret requested a review from a team as a code owner December 5, 2023 11:05
@kplattret kplattret requested a review from a team December 5, 2023 11:05
Since #228, the Python workflow will run for any changes in the Python
code, the shared test resources _and_ the Python workflow itself. This
means that when the library version does not change, the `publish` step
simply fails, which does not look good.

This updates the Python workflow so that it will only attempt to publish
the library when there is a `python/v**` tag being pushed. It does mean
that it now requires an additional manual step, but it also means that
we can more easily batch changes in each version bump, without the
workflow failing in between.
@kplattret kplattret force-pushed the python-publish-on-new-tag-only branch from 5757e30 to 7a8d9ea Compare December 5, 2023 11:29
Copy link
Contributor

@tl-flavio-barinas tl-flavio-barinas left a comment

Choose a reason for hiding this comment

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

LGTM, although I would like to avoid the manual step.
How about a script that checks if the version exists and leaves a warning annotation?

if ! VERSION="$(grep -m1 'version = "' pyproject.toml | cut -d'"' -f2)"; then exit 1; fi
STATUS_CODE=$(curl --write-out %{http_code} --silent --output /dev/null "https://pypi.org/pypi/truelayer-signing/${VERSION}/json")

if [ $STATUS_CODE -eq 200 ] ; then
    echo "::warning truelayer-signing:$VERSION::Version already exists skipping publication."
else
    poetry build
    poetry config pypi-token.pypi ${{ secrets.PYPI_API_TOKEN }}
    poetry publish
fi

@kplattret
Copy link
Member Author

@tl-flavio-barinas I don't mind at all if that's your preference. I think this should work well for whoever maintains the specific library really.

The one thing worth pointing out is that skipping the publish job altogether is clearer from a workflow perspective. As shown on the screenshot below, it's more reassuring to see that the library was not published, rather than see the step as successfully executed, then having to check or remember that it didn't actually work as there was no version bump.

image

@kplattret
Copy link
Member Author

@tl-flavio-barinas maybe we can find a way of doing your check in the job's conditional statement though, as opposed to doing it in an execution step?

@tl-flavio-barinas
Copy link
Contributor

@tl-flavio-barinas maybe we can find a way of doing your check in the job's conditional statement though, as opposed to doing it in an execution step?

It would be nice if we could filter at a job level. Not sure it's possible, though. We could rename the job to try_publish + the annotation + update the script to fail if the code has changed and the version hasn't.

@kplattret kplattret force-pushed the python-publish-on-new-tag-only branch 2 times, most recently from 40d1263 to 3ef50eb Compare December 8, 2023 17:07
@kplattret kplattret force-pushed the python-publish-on-new-tag-only branch from 3ef50eb to 90e1d85 Compare December 8, 2023 17:12
@kplattret kplattret force-pushed the python-publish-on-new-tag-only branch from a7ec69d to 947322a Compare December 8, 2023 17:20
@kplattret
Copy link
Member Author

@tl-flavio-barinas I've updated the workflow as such:

  1. Added a check_version job that will only run when pushing to the main branch and if the lint_and_test job succeeds. You can see it work here.
  2. Updated the publish job so that it only runs:
    • when pushing to the main branch;
    • if lint_and_test and check_version succeed;
    • if current version's is_published is false.

This removes the manual step of tagging and keeps the workflow elegant, with the publish job only running when necessary, instead of going red.

@kplattret kplattret changed the title Python: publish on new tag only Python: publish on version bump only Dec 8, 2023
Copy link
Contributor

@tl-flavio-barinas tl-flavio-barinas left a comment

Choose a reason for hiding this comment

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

LGTM

@kplattret kplattret merged commit 834cba7 into main Dec 11, 2023
6 checks passed
@kplattret kplattret deleted the python-publish-on-new-tag-only branch December 11, 2023 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants