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

Auto pushing to TestPypi #26

Merged
merged 11 commits into from
Oct 27, 2024
Merged

Conversation

lukepolson
Copy link
Contributor

@lukepolson lukepolson commented Sep 29, 2024

  1. I've added a pyproject.toml file to /python that has dependencies for the code generated by the yardl generate command (the python files it creates).
  2. I've updated the ci.yml file so that it auto pushes to Test Pypi. @KrisThielemans or whoever is owner of the repo, you'll have to manually add a TEST_PYPI_TOKEN as follows in order for this to work. (1) Sign up for TestPypi. (2) Create an API token on TestPyPi via Account Settings -> Add API token. (3) Add the API token on Github with the name TEST_PYPI_TOKEN via Settings -> Secrets and Variables -> Actions -> New Repository secret. Unfortunately only owners of the repo can do this.

Whenever main gets pushed to, a package with the name prd (and version specified by pyproject.toml) will get uploaded to testpypi under your account. If the version remains the same with two pushes to main, I think pushing to testpypi will fail (once we update to regular pypi it will definitely fail since it needs different version numbers every time).

By the way @KrisThielemans, this prevents us from having to manually add the generated code in /prd to the repository, which you eluded to in #23 . Essentially the workflow (i) creates the code in /prd via yardl and (ii) serves everything in /prd as its own python package on pypi.

@lukepolson
Copy link
Contributor Author

Linked to #23

@KrisThielemans KrisThielemans linked an issue Sep 29, 2024 that may be closed by this pull request
Copy link
Contributor

@KrisThielemans KrisThielemans left a comment

Choose a reason for hiding this comment

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

Great. Thanks a lot! Nice and simple. I've created the TEST_PYPI_TOKEN.

I'd prefer to have the TestPyPi upload in a different action, presumably depending if the CI action succeeded. Easy?

python/pyproject.toml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@KrisThielemans KrisThielemans self-assigned this Sep 29, 2024
@KrisThielemans KrisThielemans added the enhancement New feature or request label Sep 29, 2024
@KrisThielemans
Copy link
Contributor

Action failure

/usr/share/miniconda/envs/yardl/bin/python: No module named build

@KrisThielemans
Copy link
Contributor

@casperdcl, maybe you can have a look at this PR and add some recommendations.

@lukepolson
Copy link
Contributor Author

lukepolson commented Sep 29, 2024

Ah my bad, I forgot to install build in that code (I was testing in a different branch with many test pushes to test the workflow and had to copy a bunch of code to an earlier commit in order to not pollute git). I'll fix. And yes, I can add as a separate workflow that only runs if the CI action succeeds.

This is an example of a separate workflow in my own fork
https://github.com/lukepolson/PRDdefinition/blob/main/.github/workflows/pypi.yml

@lukepolson
Copy link
Contributor Author

lukepolson commented Sep 29, 2024

@KrisThielemans just updated and created a separate workflow which depends on ci.yml succeeding. The separate workflow also installs all the necessary packages for building the code in the /prd folder.

Once this is sufficiently tested, we'll have to create a workflow for adding to the real PyPI as well. We may not want to link this to pushing to main. (According to ChatGPT) we can also have it trigger on a new GitHub release via

on:
  release:
    types:
      - created

@KrisThielemans
Copy link
Contributor

Excellent. thanks! I'll let the experts comment.

.github/workflows/test_pypi.yml Outdated Show resolved Hide resolved
.github/workflows/test_pypi.yml Show resolved Hide resolved
.github/workflows/test_pypi.yml Show resolved Hide resolved
python/pyproject.toml Outdated Show resolved Hide resolved
python/pyproject.toml Outdated Show resolved Hide resolved
python/pyproject.toml Outdated Show resolved Hide resolved
@KrisThielemans
Copy link
Contributor

This action is currently not running. @casperdcl @johnstairs , do we need to first merge this to main before it can be debugged?

I still think having it as a separate action is the best way forward, as we will need to push to Pypi only on releases (the equivalent of https://github.com/ismrmrd/mrd/blob/main/.github/workflows/mrd_pypi.yml). It is not clear to me if a depending action will have access to files created by the CI workflow. If not, it could push the python files as an artefact.

@KrisThielemans
Copy link
Contributor

I'll merge this to see what happens. Likely will need to be improved later...

@KrisThielemans KrisThielemans merged commit a7858b3 into ETSInitiative:main Oct 27, 2024
1 check passed
@KrisThielemans
Copy link
Contributor

This sort of worked: https://test.pypi.org/project/petsird/0.0.1/

Thanks @lukepolson !

However,

$ python3 -m venv petsirdenv
$ source petsirdenv/bin/activate
(petsirdenv) $ pip install -i https://test.pypi.org/simple/ petsird==0.0.1
Collecting petsird==0.0.1
  Could not find a version that satisfies the requirement petsird==0.0.1 (from versions: )
No matching distribution found for petsird==0.0.1
(petsirdenv) $ $ pip install -i https://test.pypi.org/simple/ petsird
Collecting petsird
  Could not find a version that satisfies the requirement petsird (from versions: )
No matching distribution found for petsird

No idea what to do...

@lukepolson
Copy link
Contributor Author

@KrisThielemans this could happen if your venv doesn't match the requirements file. Are you sure you have the right version of python3 installed?

@KrisThielemans
Copy link
Contributor

oops. shows that I always use conda...
With python 3.11. I got

$ pip install -i https://test.pypi.org/simple/ petsird
Looking in indexes: https://test.pypi.org/simple/
Collecting petsird
  Downloading https://test-files.pythonhosted.org/packages/98/57/5c8a7238e3c2a50704fefd58041be5f32e05f22b9442a802829bbd7f4bf8/petsird-0.0.1-py3-none-any.whl (38 kB)
INFO: pip is looking at multiple versions of petsird to determine which version is compatible with other requirements. This could take a while.
ERROR: Could not find a version that satisfies the requirement numpy>=1.22 (from petsird) (from versions: 1.9.3)
ERROR: No matching distribution found for numpy>=1.22

I had to manually do pip install numpy first, after which it worked. Doesn't pip auto-install dependencies?

@casperdcl
Copy link
Member

casperdcl commented Oct 29, 2024

lol use --extra-index-url instead of -i

@casperdcl casperdcl mentioned this pull request Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

generate Pypi/conda packages
4 participants