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

Automate publishing Python package on PyPI #1580

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

bittner
Copy link
Contributor

@bittner bittner commented Mar 5, 2024

This PR removes the now superflous setup.py and adds 2 Tox environments that help with publishing the Python package on PyPI.

  • tox -e package can be used for linting to help detect packaging issues.
  • tox -e package -- upload allows to publish the package on PyPI.
  • tox -e ensure_version_matches $GIT_TAG verifies that the package version matches the semantic Git tag on the current commit. This is used by the GHA configuration to prevent a mismatch of tags and the version of the published package.

To make publishing work, you need to add the 2 secrets PYPI_USERNAME and PYPI_PASSWORD to Settings ➜ Secrets ➜ Actions with the PyPI access token for the project, as described in the Python packaging user guide.

Related

@bittner bittner force-pushed the feature/automate-publish-pypi branch from c584a37 to 2492b7c Compare March 5, 2024 14:44
Copy link
Collaborator

@lengau lengau 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 this!

I think snapcraft 8 lets us get rid of the setup.py file, but we'll find out when I run the CI

.github/workflows/publish-pypi.yaml Outdated Show resolved Hide resolved
@bittner bittner force-pushed the feature/automate-publish-pypi branch from 2492b7c to 9ed5e8a Compare March 6, 2024 07:28
@bittner
Copy link
Contributor Author

bittner commented Mar 6, 2024

I think snapcraft 8 lets us get rid of the setup.py file, but we'll find out when I run the CI

The snap-build job fails, because the charmcraft snap configuration takes for granted that setup.py is around:

:: ++ python3 setup.py --version
:: python3: can't open file '/root/parts/charmcraft/src/setup.py': [Errno 2] No such file or directory

From a Python (packaging) point of view the use of setup.py is legacy and every modern Python package should switch over to using (just) pyproject.toml. I guess it's okay if I try to fix the snap configuration with an additional commit to make the pipeline pass.

@bittner bittner force-pushed the feature/automate-publish-pypi branch from 9ed5e8a to f40db10 Compare March 6, 2024 22:47
@bittner
Copy link
Contributor Author

bittner commented Mar 15, 2024

The snap-build job fails as it can't find the craft_cli module.

ModuleNotFoundError: No module named 'craft_cli'

I can't see that the craft-cli Python package is installed in the job, I can only see it as a dependency in requirements.txt.

Can anyone see what is the problem?

@bittner
Copy link
Contributor Author

bittner commented Mar 15, 2024

Can anyone see what is the problem?

Okay, looks like there is a problem with the sed command before that:

sed: -e expression #1, char 14: unterminated `s' command

The problem is that that with the new setup the returned version string also contains the package name. I'll try to find a solution. Suggestions welcome!

EDIT: Stupid me! A closing / was missing. 🤪

@bittner bittner force-pushed the feature/automate-publish-pypi branch 5 times, most recently from 98ab399 to b1bf7dc Compare March 16, 2024 15:21
@bittner
Copy link
Contributor Author

bittner commented Mar 16, 2024

The CI jobs all pass now (on my personal fork). I rebased the branch, and I made separate commits to document the specific changes that were necessary to work around a few build issues. 📜

Hope that looks good enough now for the PR to be merged. 🤞

@lengau lengau self-requested a review March 18, 2024 17:13
@bittner bittner force-pushed the feature/automate-publish-pypi branch from b1bf7dc to f7b9151 Compare March 18, 2024 18:16
@bittner
Copy link
Contributor Author

bittner commented Mar 18, 2024

I have signed the CLA now. I use a different email address on Launchpad than on GitHub, that's why the check probably didn't find my Launchpad account.

@bittner
Copy link
Contributor Author

bittner commented Mar 18, 2024

The cla-check still fails. I checked my Launchpad account again; looks like there was another account with the email address I use on GitHub. I merged the accounts on Launchpad now.

@bittner
Copy link
Contributor Author

bittner commented Mar 19, 2024

I received an email notification this morning saying, I was added as a member of Canonical Contributor Agreement.

I assume, the cla-check job would now succeed.

pyproject.toml Show resolved Hide resolved
@@ -138,7 +138,7 @@ parts:
# Ensure we don't have a dubious ownership error from git with a remote build.
git config --global --add safe.directory $CRAFT_PART_SRC
# set the version
version="$(python3 setup.py --version)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This change seems to be always making the charmcraft snap that gets built report the version dev, I'm guessing because it's not recognised as installed at this point, so there's not a _version.py file in the charmcraft directory from which to report.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't setuptools-scm take care of generating the _version.py file, transparently? Have you seen any issues in the build process? 😲

Copy link
Collaborator

Choose a reason for hiding this comment

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

The snap artefact from the build is reporting that, and I've been unable to come up with something else that makes it work as intended. Could you revert the removal of setup.py for now, since having it there shouldn't break things?

@bittner
Copy link
Contributor Author

bittner commented Mar 19, 2024

@lengau Let me know what needs to be changed, if anything, so that I can help getting this merged. Thank you! 👍

@bittner bittner force-pushed the feature/automate-publish-pypi branch 2 times, most recently from b93b823 to c4dd4a3 Compare March 22, 2024 12:54
@bittner
Copy link
Contributor Author

bittner commented Mar 22, 2024

I rebased the branch of this PR again to keep it up-to-date with the main branch.

If the changes look okay, I'd be happy to see this merged. Otherwise, please let me know what needs to be changed.

@bittner bittner force-pushed the feature/automate-publish-pypi branch from c4dd4a3 to a6679f7 Compare March 25, 2024 19:47
The GHA process expects a semantic Git tag and verifies that the package
version matches that tag, before publishing the package on PyPI.
@bittner bittner force-pushed the feature/automate-publish-pypi branch from a6679f7 to e6cc956 Compare March 31, 2024 20:21
@bittner
Copy link
Contributor Author

bittner commented Apr 1, 2024

Is there a reason why this PR isn't considered for merging? Any arguments against the changes? Anything that needs to be updated?

It would be very helpful if the recent release(s) of charmcraft were published also on PyPI. For example, I'm currently blocked by the No keyring found error that was fixed in version 2.3.0 by allowing anonymous downloads of charm library modules. As explained in #1579 (comment) a pure-Python installation is needed for the CI use case.

Copy link
Collaborator

@lengau lengau left a comment

Choose a reason for hiding this comment

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

Please re-add setup.py, and then I'm happy with this :-)

I've created #1635 for seeing if we can remove setup.py at a later date so we don't block this PR with it

@@ -1,4 +1,4 @@
attrs==23.2.0
attrs==22.2.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Version downgrade?

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.

None yet

3 participants