cookiecutter-hypermodern-python icon indicating copy to clipboard operation
cookiecutter-hypermodern-python copied to clipboard

Release workflow fails on code import if TEST_PYPI_TOKEN is unset

Open cjolowicz opened this issue 3 years ago • 11 comments

The initial GitHub upload of a generated project can result in a broken build, because the Release workflow will attempt to upload the package to TestPyPI, and the TEST_PYPI_TOKEN may not have been set yet.

There are two things we can do about this:

  • Document that tokens should be set up before pushing.
  • Make the TestPyPI upload conditional on TEST_PYPI_TOKEN being set.

Related questions:

  • Which other external services can be set up before pushing, and which should be?
  • Which other CI steps can be conditional on services being set up, and which should be?

cjolowicz avatar Jul 09 '20 10:07 cjolowicz

@cjolowicz there is this new manual triggers that could put the workflow/commit in a pending state if TEST_PYPI_TOKEN or even PYPI_TOKEN and CODECOV_TOKEN are not set. One could skip it or insert manually these tokens so that the workflow completes successfully.

staticdev avatar Jul 20 '20 16:07 staticdev

That's an interesting idea. But AFAIU manual triggers apply to the entire workflow, not an individual step. They also don't support additional conditions. Or am I missing something here?

cjolowicz avatar Aug 13 '20 20:08 cjolowicz

I also didn't dig deep into that.. I am used to manual steps in jenkins and gitlab-ci, so I thought it would be the same in GA. But I see you found a simple and clever solution.

staticdev avatar Aug 13 '20 23:08 staticdev

Unfortunately steps cannot be conditional on the presence of a secret, see #512. We might still document the correct flow to set up a repository without creating a failed workflow run on import.

cjolowicz avatar Aug 14 '20 19:08 cjolowicz

A workaround would be to copy the secret to an environment variable first, see https://github.com/actions/runner/issues/520#issuecomment-860043020. As pointed out later in that thread, this could result in leaking the secret in debug runs, so maybe this is not an option.

cjolowicz avatar Jun 15 '21 13:06 cjolowicz

@cjolowicz that depends on the threat model we are using. But for most cases, test pypi credentials shouldn't be critical and it is not that it IS leaking, but can possibly leak. Seems safe to me.

staticdev avatar Jun 15 '21 16:06 staticdev

Small addendum... the PyPA workflow action guidelines recommend naming it PYPI_API_TOKEN with API in there.

Also, I don't see a mention in the docs about adding the pypi- prefix to the secret?

pauleveritt avatar Nov 06 '21 14:11 pauleveritt

Hi @pauleveritt thanks for chiming in here! I'm hesitant to change the secret name to PYPI_API_TOKEN, as it would potentially break people who keep their existing projects up-to-date with the template, using tools such as cruft. I agree that using the same name as the PyPA recommendation reduces friction for people who already follow it in their projects. On the other hand, I wouldn't see the name as part of the "normative part" of the guide.

As for the pypi- prefix, I'm not sure I understand. Isn't that just part of the API token as displayed by PyPI?

cjolowicz avatar Nov 08 '21 10:11 cjolowicz

Reasonable point and thanks for pointing me at cruft. Ignore the pypi- part, this was me making a mistake with getting the token.

Thanks so much for the work on this cookiecutter and especially the docs and blog post series.

pauleveritt avatar Nov 08 '21 12:11 pauleveritt

You're welcome!

cjolowicz avatar Nov 10 '21 04:11 cjolowicz

A workaround would be to copy the secret to an environment variable first, see actions/runner#520 (comment). As pointed out later in that thread, this could result in leaking the secret in debug runs, so maybe this is not an option.

See also https://github.community/t/if-expression-with-context-variable/16558

cjolowicz avatar Nov 22 '21 12:11 cjolowicz