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

Modify github-to-sops directory structure for publishing to PyPI #3

Merged
merged 11 commits into from
Feb 8, 2024

Conversation

rjwignar
Copy link
Collaborator

@rjwignar rjwignar commented Jan 29, 2024

Summary

This Pull Request modifies the directory structure of github-to-sops to one that can be packaged and published to PyPI.
This was done using Simon Willison's cookiecutter template to create a mock project and replicating mock project's directory structure in this branch.

These changes should allow us to ship github-to-sops to ChatCraft via pip, as per tarasglek/chatcraft.org#324

The cookiecutter template

The cookiecutter template:

  • creates a pyproject.toml file configured with setuptools
    • includes pytest as a test dependency
  • adds a README
  • creates package_name/__init__.py
  • creates tests/test_package_name.py
  • Adds two workflows in .github/workflows:
    • one to run tests against Python 3.8 to 3.12
    • one to publish package releases to PyPI.

My modifications

  • added pyproject.toml based on what is produced by the template. Content modified to be specific to github-to-sops
  • Kept original README.md
  • moved github-to-sops to github-to-sops/__init__.py
  • added tests/test_github_to_sops with empty tests
  • copied .github/workflows to directory (as the workflow content doesn't depend on the project name)

Next Steps

Assuming this directory structure is correct, the next steps would be to use PyPI's Trusted Publishing mechanism to publish github-to-sops to PyPI.

Below is an example of the Trusted Publishing form Willison filled to publish his own library, datasette-test
image

We would have to fill in this form with this repo's info.
I might be able to fill in the form myself but I wanted to first confirm these changes with @tarasglek.

@rjwignar
Copy link
Collaborator Author

rjwignar commented Jan 29, 2024

I was a little iffy on the tests folder. Should I remove it and the reference to pytest in pyproject.toml?
Removing tests would also make .github/workflows/test.yml unnecessary.

Copy link
Collaborator

@humphd humphd left a comment

Choose a reason for hiding this comment

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

@rjwignar great work! Thanks for pushing on this for 1.1.

I'm not a Python dev, but I wonder if this is right github-to-sops → github-to-sops/__init__.py. I'd be surprised to find the source for a module in __init__.py. Maybe @tarasglek knows.

How do secrets work for the CI publish workflow? I assume @tarasglek has to do this from his account somehow?

pyproject.toml Outdated Show resolved Hide resolved

Choose a reason for hiding this comment

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

I am uncertain whether we should add the most basic test?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree. I think I will remove this file.
Depending on if we want tests at all, I might have to remove or modify some other files that reference pytest
(e.g. .github/workflows/test.yml).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed tests/test_github-to-sops as of 278bda4, leaving the tests folder empty.
The test.yml workflow still passes without errors: https://github.com/rjwignar/github-to-sops/actions/runs/7735949309

@tarasglek, Simon's template configures the library with pytest as an optional dependency.
We currently don't have any tests setup for github-to-sops, but I see unit tests are something you'd like to see added to github-to-sops:
image

With that in mind, would you like this PR to be configured with pytest setup so tests can be added in the future?
i.e. keep the following in the PR?:

  • an empty test folder
  • .github/workflows/test.yml,
  • the test jobs inside publish.yml,
  • pytest as an optional dependency in pyproject.toml)

pip install '.[test]'
- name: Run tests
run: |
pytest

Choose a reason for hiding this comment

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

why this step is different from the Run tests in test.yml?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure why. Both workflows were generated by https://github.com/simonw/python-lib-template-repository.
However, we can remove this step entirely if we don't include pytest in this library.

@rjwignar
Copy link
Collaborator Author

rjwignar commented Jan 31, 2024

@kliu57 could you take a look at github-to-sops/__init__.py?
I moved the python library from the file github-to-sops to github-to-sops/__init__.py.

I've seen some projects with empty an __init__.py, and some with __init__.py containing programs.

Does it make sense to have the github-to-sops program entirely inside __init__.py?

@humphd
Copy link
Collaborator

humphd commented Jan 31, 2024

@tarasglek can you add @rjwignar to the collaborators for this repo?

@kliu57
Copy link

kliu57 commented Jan 31, 2024

@kliu57 could you take a look at github-to-sops/__init__.py? I moved the python library from the file github-to-sops to github-to-sops/__init__.py.

I've seen some projects with empty an __init__.py, and some with __init__.py containing programs.

Does it make sense to have the github-to-sops program entirely inside __init__.py?

Back when I published something to PyPI last term I used an empty __init__.py file. See blog to see what I did and what instructional video I followed. Did you read some instructions from somewhere which told you to put the program inside __init__.py?

@rjwignar
Copy link
Collaborator Author

rjwignar commented Jan 31, 2024

@kliu57 could you take a look at github-to-sops/__init__.py? I moved the python library from the file github-to-sops to github-to-sops/__init__.py.
I've seen some projects with empty an __init__.py, and some with __init__.py containing programs.
Does it make sense to have the github-to-sops program entirely inside __init__.py?

Back when I published something to PyPI last term I used an empty __init__.py file. See blog to see what I did and what instructional video I followed. Did you read some instructions from somewhere which told you to put the program inside __init__.py?

The directory structure generated by https://github.com/simonw/python-lib-template-repository includes program code inside __init__.py. I moved the github-to-sops code to __init__.py based on that.

Here's the __init__.py generated by the template:
https://github.com/rjwignar/test/blob/main/test/__init__.py

@kliu57
Copy link

kliu57 commented Jan 31, 2024

@kliu57 could you take a look at github-to-sops/__init__.py? I moved the python library from the file github-to-sops to github-to-sops/__init__.py.
I've seen some projects with empty an __init__.py, and some with __init__.py containing programs.
Does it make sense to have the github-to-sops program entirely inside __init__.py?

Back when I published something to PyPI last term I used an empty __init__.py file. See blog to see what I did and what instructional video I followed. Did you read some instructions from somewhere which told you to put the program inside __init__.py?

The directory structure generated by https://github.com/simonw/python-lib-template-repository includes program code inside __init__.py. I moved the github-to-sops code to __init__.py based on that.

Here's the __init__.py generated by the template: https://github.com/rjwignar/test/blob/main/test/__init__.py

I see, so it was automatically generated by the tool. I've never used this tool, I did my project restructuring manually.

@humphd
Copy link
Collaborator

humphd commented Jan 31, 2024

If Simon puts the code in __init__.py, it must be the right way to do it--he publishes tons of pypi stuff. Let's not overthink it.

@rjwignar
Copy link
Collaborator Author

rjwignar commented Feb 1, 2024

I've been able to fill out the form myself and add github-to-sops as a pending project on PyPI (we've reserved the package name github-to-sops)

How do secrets work for the CI publish workflow? I assume @tarasglek has to do this from his account somehow?

@humphd It looks like no secrets are involved in the publish workflow.

From Simon's blog on Trusted Publishing, It looks like Trusted Publishing involves three steps:

  • Tell PyPI which GitHub repository should be allowed to publish a package with a specific name
    • Completed by filling out the Pending Publisher form:
      image
    • note that release is the configured publishing environment name
  • Configure GitHub Actions publish workflow to use the pypa/gh-action-pypi-publish@release/v1 action
    • .github/workflows/publish.yml generated by Simon's template
    • note that release is configured as the environment name in the deploy job of publish.yml, to match the Pending Publisher setup:
      deploy:
      runs-on: ubuntu-latest
      needs: [test]
      environment: release
      permissions:
      id-token: write
  • Publish a release to GitHub that triggers the workflow
    • Publishing releases is restricted to Taras unless anyone else is granted Write access to the repo.

@rjwignar
Copy link
Collaborator Author

rjwignar commented Feb 1, 2024

If Simon puts the code in __init__.py, it must be the right way to do it--he publishes tons of pypi stuff. Let's not overthink it.

I've taken a look at some of his recent projects and he keeps source code into submodules that are imported by __init__.py.
However, keeping all the code in __init__.py is possible too. pylons/colander has its core code (~3000 lines) inside __init__.py!

I think for this PR, we should keep __init__.py as is, then we can look into moving the code into submodules in a follow-up issue.

- removed [project.optional-dependencies] from pyproject.toml
- removed test.yml workflow
- removed test job and test job dependency from publish.yml
@tarasglek
Copy link
Owner

Invited you guys to the repo

@tarasglek
Copy link
Owner

@rjwignar I don't think you need anything from me to proceed, please push a release.

@rjwignar rjwignar merged commit bd91401 into tarasglek:main Feb 8, 2024
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

5 participants