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

Fix: update release workflow to follow pypa and pypi guidelines #73

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# pyOpenSci packaging template Changelog

## [Unreleased]

* Update release workflow to follow PyPA / PyPI recommended practices (@lwasser, #48)
15 changes: 15 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# Contributing to the pyOpenSci Python package template

To work on the template locally, you can call the copier template directly. Note that by default, `copier` uses the latest tag in your commit history. To ensure it uses the latest commit on your current active branch use:

`copier copy -r HEAD /path/to/your/template destination-dir`

If you want to test it against the latest tag in your local commit history, you can use:

`copier copy /path/to/your/template destination-dir`

## Run the tests

You can use Hatch to run all of the tests for the template:

`hatch run test:run`
22 changes: 11 additions & 11 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ To use this template:
1. Install copier using [pipx](https://pipx.pypa.io/stable/) or pip preferably with a [virtual environment](https://www.pyopensci.org/python-package-guide/CONTRIBUTING.html#create-a-virtual-environment).

Global Installation:

```console
pipx install copier
```

or Environment specific installation:

```console
Expand All @@ -37,8 +37,9 @@ To use this template:
```console
copier copy gh:pyopensci/pyos-package-template path/here
```
The command below will create the package directory in your current working directory.

The command below will create the package directory in your current working directory.

```console
copier copy gh:pyopensci/pyos-package-template .
```
Expand All @@ -48,14 +49,13 @@ To use this template:
as your source. You can read more about generating your project
in the [copier documentation](https://copier.readthedocs.io/en/stable/generating/).


## Run the template workflow

Once you have installed copier, you are ready to create your Python package template.
First, run the command below from your favorite shell. Note that this is copying our template from GitHub so it
Once you have installed copier, you are ready to create your Python package template.
First, run the command below from your favorite shell. Note that this is copying our template from GitHub so it
will require internet access to run properly.

The command below will create the package directory in your current working directory.
The command below will create the package directory in your current working directory.

`copier copy gh:pyopensci/pyos-package-template .`

Expand All @@ -64,13 +64,13 @@ If you wish to create the package directory in another directory you can specify
`copier copy gh:pyopensci/pyos-package-template dirname-here`

## Template overview
The copier template will ask you a series of questions which you can respond to. The questions will
help you customize the template.

The copier template will ask you a series of questions which you can respond to. The questions will
help you customize the template.

Below is what the template workflow will look like when you run it. In the example below, you
"fully customize" the template.


```console
copier copy gh:pyopensci/pyos-package-template .

Choose a reason for hiding this comment

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

By the way, I noticed that this file has a few trailing whitespace in a few lines, you might consider fixing them with a local pre-commit hook: https://github.com/pre-commit/pre-commit-hooks#trailing-whitespace. For example, you fixed one such case yourself in line 114 of copier.yml 🙈 (not with this PR, of course!)

🎤 Who is the copyright holder, for example, yourself or your organization? Used in the license
Expand Down
4 changes: 2 additions & 2 deletions copier.yml
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ documentation:
type: str
help: "Do you want to include documentation for your project and which framework do you want to use?"
choices:
"Sphinx (https://www.pyopensci.org/pyos-sphinx-theme)": sphinx
"Sphinx (https://pydata-sphinx-theme.readthedocs.io/en/stable/index.html)": sphinx
Copy link
Member Author

Choose a reason for hiding this comment

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

This is just a tiny change - pyos-sphinx-theme is specific to our guidebooks. so let's go with the community lead pydata-sphinx-theme instead 🚀 :)

"mkdocs-material (https://squidfunk.github.io/mkdocs-material)": mkdocs
No: ""
default: "{% if use_default != 'minimal' %}sphinx{% else %}{% endif %}"
Expand Down Expand Up @@ -111,7 +111,7 @@ license:
type: str
help: |
Which license do you want to use? Includes a LICENSE file in the repository root.
For more information, see:
For more information, see:
- https://www.pyopensci.org/python-package-guide/documentation/repository-files/license-files.html
- https://opensource.org/licenses
choices:
Expand Down
2 changes: 1 addition & 1 deletion template/pyproject.toml.jinja
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ dependencies = [
detached = true

[tool.hatch.envs.style.scripts]
docstrings = "pydoclint"
docstrings = "pydoclint src/ tests/"
code = "ruff check {args}"
format = "ruff format {args}"
check = ["docstrings", "code"]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,53 +1,77 @@
name: CD
name: Publish to PyPI

on:
push:
tags:
- 'v?[0-9]+.[0-9]+.[0-9]+'
- 'v?[0-9]+.[0-9]+.[0-9]+(a|b|rc|post|dev)[0-9]+'
release:
types: [published]

jobs:
prerequisites:
uses: ./.github/workflows/test.yml

release:
# Setup build separate from publish for added security
# See https://github.com/pypa/gh-action-pypi-publish/issues/217#issuecomment-1965727093
build:
needs: [prerequisites]
Copy link
Member Author

Choose a reason for hiding this comment

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

@Midnighter i added the test run back -- here before we build the package.

strategy:
matrix:
os: [ubuntu-latest]
python-version: ["3.12"]
runs-on: ${{ matrix.os }}
permissions:
# Write permissions are needed to create OIDC tokens.
id-token: write
# Write permissions are needed to make GitHub releases.
contents: write

runs-on: ubuntu-latest
# Environment is encouraged for increased security
environment: build
Comment on lines +15 to +16

Choose a reason for hiding this comment

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

This is a new one for me, so I'm curious. Could you please share how it works?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh! @agriyakhetarpal environments allow you to setup a trusted connection with pypi. This is the build environment, so we don't need to worry too much about security here in terms of PYPI. so anyone can run the build step in theory. but by having a publish environment, we can control who can trigger that step of the build. (if that is what you're asking) .

here is a bit more about this

https://docs.pypi.org/trusted-publishers/using-a-publisher/

Choose a reason for hiding this comment

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

Oh, I'm aware of trusted publishing and environments, thanks! My question was more around the need for this build environment, as it seems to be redundant. Most release workflows can get by with just one pypi (or publish, or any other similar name) environment, omitting the need for an environment for the build step. Does including two environments, one for building and one for publishing, have security provisions? Moreover, I imagine that the users would also have questions around why they need (and we recommend) to set up two environments in their repository's settings, when it is only the publishing one that connects to PyPI's OIDC publishing functionalities.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh gosh my apologies. i suspected I was telling you something you already knew. 🙈

This is a great point. I don't know the answer to your question!! But i suspect this is ME introducing complexity.

i just checked the tutorials and there is a build step but not an additional environment. Shall we remove this environment as you are correct it's not required. we need the job but not an environment AFAIK .

Choose a reason for hiding this comment

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

Oh gosh my apologies. i suspected I was telling you something you already knew. 🙈

No worries at all! 😄

This is a great point. I don't know the answer to your question!! But i suspect this is ME introducing complexity.

i just checked the tutorials and there is a build step but not an additional environment. Shall we remove this environment as you are correct it's not required. we need the job but not an environment AFAIK .

Thank you for dropping it!

steps:
- uses: actions/checkout@v4

- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python-version }}
- name: Checkout
uses: actions/checkout@v4
with:
# This fetch element is only important if you are use SCM based
# versioning (that looks at git tags to gather the version)
fetch-depth: 100
# Need the tags so that setuptools-scm can form a valid version number
- name: Fetch git tags
run: git fetch origin 'refs/tags/*:refs/tags/*'
Comment on lines +18 to +26

Choose a reason for hiding this comment

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

This can be simplified with the fetch-tags: input, which fetches all tags without needing to fetch up to a higher depth.

Suggested change
- name: Checkout
uses: actions/checkout@v4
with:
# This fetch element is only important if you are use SCM based
# versioning (that looks at git tags to gather the version)
fetch-depth: 100
# Need the tags so that setuptools-scm can form a valid version number
- name: Fetch git tags
run: git fetch origin 'refs/tags/*:refs/tags/*'
- name: Checkout
uses: actions/checkout@v4
with:
# This fetch element is only important if you are use SCM based
# versioning (that looks at git tags to gather the version).
# setuptools-scm needs tags to form a valid version number
fetch-tags: true

I have to note that this isn't working with ubuntu-24.04 images, though (actions/checkout#2041), but it looks like it will be resolved soon, well before ubuntu-latest starts pointing to 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.

ahhhh ok perhaps we hold off if this is bleeding edge and not vetted across platforms. As we do want our users to have a happy experience creating their first package! It's more important that this template is easy to use and understand!

Choose a reason for hiding this comment

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

Oh, it's not an entirely new feature (it calls a variant of git fetch underneath), actually. The fix will be rolled out soon enough, reducing one step here, so I'll suggest we keep these, but I'm fine with waiting until we know it's working everywhere!

Copy link
Member Author

Choose a reason for hiding this comment

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

cool! Thanks, @agriyakhetarpal. What do you think about us moving this to an issue and applying when the bugs have been ironed out on Linux/Ubuntu? All of your comments are awesome BTW - THANK YOU for taking the time to review. I'm really excited to teach with this template in the spring!

Choose a reason for hiding this comment

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

Sounds good to me! I'll remember to open an issue that links back to this review thread, once this PR is merged. The pleasure is all mine!


- name: Install hatch
uses: pypa/hatch@install
- name: Setup Python
uses: actions/setup-python@v5
with:
# You can modify what version of Python you want to use for your release
python-version: "3.11"

- name: Build package
run: hatch build
# Security recommends we should pin deps. Should we pin the workflow version?
Copy link

@agriyakhetarpal agriyakhetarpal Feb 27, 2025

Choose a reason for hiding this comment

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

Yes, please. I'll recommend that all GitHub Actions—whether by GitHub as an actor or external ones available on the marketplace—should be pinned to their commit hashes, at least for a workflow concerning a release. There is a case to be made about how it makes things a bit more challenging for new contributors/packagers, but I think supply-chain security shouldn't be at the end of a compromise.

There are tools such as gha-update which can replace them with pins, that we can recommend elsewhere (not in this PR, though, of course). Once this is done, I think it's just a matter of letting Dependabot (if configured) take over, as it can handle both the upgrade and the inline version comment in its automated PRs.

Copy link
Member Author

Choose a reason for hiding this comment

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

great. So, looking at the action here

Ofek suggests that too - we'd use this - this hash is almost the most recent but the most recent is just a readme update.

  • name: Install Hatch
    uses: pypa/hatch@a3c83ab

    But now we'd have to update that hash manually? How often should that update happen?

We should do that as part of this repo's maintenance, but also, we'll want to tell users to update this periodically, right?

Choose a reason for hiding this comment

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

great. So, looking at the action here

Ofek suggests that too - we'd use this - this hash is almost the most recent but the most recent is just a readme update.

  • name: Install Hatch
    uses: pypa/hatch@a3c83ab
    But now we'd have to update that hash manually? How often should that update happen?

We should do that as part of this repo's maintenance, but also, we'll want to tell users to update this periodically, right?

Updating it weekly for our maintenance could be a good cadence; what do you think? It would depend on the amount of activity we have on this template.

If we were to ask our users to enable Dependabot in the GitHub settings and inherit the same settings, they should also get the same updates. However, I'm not sure whether it's too much to ask for beginners (😕).

As long as it is pinned for a start, we can proceed with a small guide on how to keep this workflow maintained, where we can describe this more (or link to an external guide if that's alright and vetted by us?).

Copy link
Member Author

Choose a reason for hiding this comment

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

@agriyakhetarpal this is awesome. We, I think, have Depdendabot set up in the org, I think, but I didn't configure it. This is a basic question but would it help update things like this version? IE could we use it in this repo?

My gut tells me we won't have the bandwidth to perform weekly updates here unless someone wants to take that one! But perhaps monthly or quarterly would work. I am not sure what is best, however and would love suggestions.

also if you were interested in helping us maintain here, that would be awesome (only if you have bandwidth!)

Choose a reason for hiding this comment

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

We, I think, have Depdendabot set up in the org, I think, but I didn't configure it. This is a basic question but would it help update things like this version? IE could we use it in this repo?

Oh, I notice that here are a few moving parts, here. As this file is not a workflow of its own but a template that will be generated and moved into .github/workflows/, I am not sure if Dependabot will be able to operate on the template file as well, in addition to the workflow file. It does have a directory: setting, which I think we should try out: https://docs.github.com/en/code-security/dependabot/working-with-dependabot/dependabot-options-reference#directories-or-directory--

That means that we have two possible scenarios:

  • the GitHub Actions dependencies in this file get outdated and are not kept up to date: assuming we also ship a Dependabot configuration file with our template that will be copied (and we instruct users on how to use Dependabot), the generated workflow file will receive updates. This should be good for users, as they receive updates. However, we, as the ones shipping the template, would need to find a method to "mirror" the updates from our workflow file to the template file.
  • the GitHub Actions dependencies in this file are also updated in addition to our own workflows (that would be great!)

The first one is surely not ideal, so we should explore if Dependabot will choose to be the star here or not 😉

My gut tells me we won't have the bandwidth to perform weekly updates here unless someone wants to take that one! But perhaps monthly or quarterly would work. I am not sure what is best, however and would love suggestions.

I agree with you. Monthly updates would be nicer – and users can always bump it to daily, or weekly, and so on, based on their bandwidth.

also if you were interested in helping us maintain here, that would be awesome (only if you have bandwidth!)

I'd love to! I've also helped out with the development and maintenance of especially other copier templates, such as https://github.com/pybamm-team/pybamm-cookie – where my experience should be transferable enough for me to help out. I do have limited bandwidth, however and can't say I can work full-time on this, but I would be open to lend a hand with more reviews and occasional updates anytime :D

- name: Install hatch
uses: pypa/hatch@a3c83ab3d481fbc2dc91dd0088628817488dd1d5

# We rely on a trusted publisher configuration being present on PyPI,
# see https://docs.pypi.org/trusted-publishers/.
- name: Publish to PyPI
uses: pypa/gh-action-pypi-publish@release/v1
- name: Build package using Hatch
run: |
hatch build
echo ""
echo "Generated files:"
ls -lh dist/
- name: GH release
uses: softprops/action-gh-release@v2
with:
body: >
Please see
https://github.com/${{ github.repository }}/blob/${{ github.ref_name }}/CHANGELOG.md
for the full release notes.
draft: false
prerelease: false
# Store an artifact of the build to use in the publish step below
- name: Store the distribution packages
uses: actions/upload-artifact@v4
with:
name: python-package-distributions
path: dist/
if-no-files-found: error
publish:
name: >-
Publish Python 🐍 distribution 📦 to PyPI
# Modify the repo name below to be your project's repo name.
if: github.repository_owner == "{{ username }}"
needs:
- build
runs-on: ubuntu-latest
# Environment required here for trusted publisher
environment:
name: pypi
# Modify the url to be the name of your package
url: https://pypi.org/p/${{ package_name }}
permissions:
id-token: write # this permission is mandatory for PyPI publishing
steps:
- name: Download dists
uses: actions/download-artifact@v4
with:
name: python-package-distributions
path: dist/
merge-multiple: true
- name: Publish package to PyPI
# Only publish to real PyPI on release
if: github.event_name == 'release'
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this line really needed since the whole workflow only triggers on release?

Choose a reason for hiding this comment

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

I would suggest including it, as (in my opinion) CD/release-related workflows like this are also supposed to run on a periodic basis using a CRON job (with a schedule: trigger) to see if the build part does not have any issues. This applies much more for packages with compiled codebases where a host of errors from the entirety of their toolchains and supported platforms are the norm, but is also applicable for almost any release workflow, i.e., even for a pure Python package – its dependencies can break, build failures can occur, and so on.

If at all, I would suggest making it more stringent, like this:

Suggested change
if: github.event_name == 'release'
if: github.event_name == 'release' && github.event.action == 'published'

in the hope of making a release-oriented developer more careful (similar to my point above) to check things twice.

Copy link
Member Author

Choose a reason for hiding this comment

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

so this is a triple check. i'm going to call in @webknjaz - Sviatoslav this is a template for pyOpenSci that people can use to quickly create a package structure. I Updated the GitHub action part to reflect what PyPA suggests in terms of security. Here we are having a small friendly debate around how many event checks should go into the build. I'm curious what your take on this is?

More can't hurt, I would presume. it just adds a bit of complexity to the build to explain to people. So i want to make sure we keep things simple yet safe! thank you!!

uses: pypa/gh-action-pypi-publish@release/v1
9 changes: 8 additions & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
"""Ruff is forcing me to write a docstring for conftest.py."""
"""Fixtures used in our test suite for the pyOpenSci Python package
template."""

import shutil
from pathlib import Path
Expand All @@ -16,6 +17,7 @@
COPIER_CONFIG_PATH = Path(__file__).parents[1] / "copier.yml"
INCLUDES_PATH = Path(__file__).parents[1] / "includes"


def _load_copier_config() -> dict:
yaml = YAML(typ="safe")
with COPIER_CONFIG_PATH.open("r") as yfile:
Expand All @@ -29,6 +31,7 @@ def _load_copier_config() -> dict:
# pytest hooks
# --------------------------------------------------


def pytest_addoption(parser: "Parser") -> None:
"""Add options to pytest."""
parser.addoption(
Expand All @@ -40,10 +43,12 @@ def pytest_addoption(parser: "Parser") -> None:
"otherwise, use a temporary directoy and remove it afterwards.",
)


# --------------------------------------------------
# Fixtures - autouse
# --------------------------------------------------


@pytest.fixture(scope="session", autouse=True)
def cleanup_hatch_envs(
pytestconfig: pytest.Config,
Expand All @@ -67,10 +72,12 @@ def cleanup_hatch_envs(
finally:
shutil.rmtree(hatch_dir, ignore_errors=True)


# ---------------------------------------------
# Fixtures - exports
# ---------------------------------------------


@pytest.fixture(scope="session")
def monkeypatch_session() -> Generator[MonkeyPatch, None, None]:
"""Monkeypatch you can use with a session scoped fixture."""
Expand Down