-
Notifications
You must be signed in to change notification settings - Fork 6
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
Generate manylinux wheels #13
base: master
Are you sure you want to change the base?
Generate manylinux wheels #13
Conversation
Serialise plasma source
Ensures we use the installed version of the package
Avoids having to re-version on each push
This is just what we needed, thanks for figuring all this out I would be tempted to put the published stuff back in but perhaps with these usernames PYPI_USERNAME and password PYPI_PASSWORD which @makeclean has added to the repository settings
|
Does it need something like so that the uploading only triggers when merging into master. Also I think we need to rename master to main branch at some point
|
@DanShort12 I can confirm that any has added github secrets PYPI_USERNAME and PYPI_PASSWORD |
The accounts used would depend on where we're uploading to. This snippet uploads to TestPyPI so it may not be the case that this shares credentials with production PyPI - although of course the same credentials could be used for both repos, in which case the same secrets could be used. I'm also not sure if a suitable account exists on TestPyPI (I used an account that I created myself when I checked the upload to TestPyPI). |
I don't think we'd want this exactly - we still want to be able to run the rest of the workflow on a PR, otherwise the build and tests won't be applied to PRs, so there won't be any checks. I think we'd want something like the below so that the upload only happens on a tag:
around the "Publish wheel artifact" action. PyPA actually have a guide with a suggestion of how to do that, which we could adopt (to upload to TestPyPI if the action isn't a tag, or to upload to PyPI if the action is a tag). However, that still raises the question of how the PyPI packages will be versioned. Since TestPyPI and PyPI need unique versions for each upload we'd need some logic to generate clean versions on upload to PyPI and unique (timestamped?) versions when uploading to TestPI. How would you prefer to do that? |
In the past I've used To upload to PyPi instead of TestPyPi we could replace
with
But I see the example uploads to TestPyPi every time and PyPi only if the commit is tagged which is a bit nicer.
But then we get the version number problem. Do you want to use the patch part of the version number for PyPi and keen major and minor increments for PyPi uploads |
I think we it will be tricky to use semantic versioning for the PR part, if pushes to a PR are going to generate an upload to TestPyPI - I think it's fair to assume that a developer wouldn't remember to bump a version number each time they are ready to push. Also IMO a PR shouldn't correspond to a new released version of the code - it's a potential new released version, but the actual released versions should align to tags in git (hence why PyPA suggest to only upload to PyPI on a new tag) e.g. a released version should be version x.y.z but a pre-release version in a PR should be x.y.z-some-metadata. If that's a suitable versioning method then we could just use the version described by git?
That gives the release version if the commit corresponds to a tag (if annotated, which it should be for a release), else it gives the number of commits since the last release and the short git hash of the commit. Basically this. That seems like a nice way to keep versions distinct for TestPyPI, while still having some control over tagging actual releases. |
Formats the version in a way that handles Python versioning standard (PEP 440) by appending the number of commits since the last tag to a dev pre-release identifier. Allows override if release not yet found (we don't have tags yet).
Also fix publish commands
This allows TestPyPI to understand the ordering of versions.
Hi @makeclean, after a bit of GitHub wrangling I think this is nearly ready to run with a full CI chain (build, test, publish). Would it be possible to create a TestPyPI account that can be used to upload new versions of the package to TestPyPI when working under a PR? We would then use that account with the TEST_PYPI_USERNAME and TEST_PYPI_PASSWORD secrets to be able to upload new packages. |
.github/workflows/python.yml
Outdated
TWINE_PASSWORD: ${{ secrets.PYPI_PASSWORD }} | ||
run: | | ||
python3 -m pip install twine | ||
python3 -m twine upload dist/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could add this, just to provide more info should it ever break
python3 -m twine upload dist/* | |
python3 -m twine upload dist/* --verbose |
.github/workflows/python.yml
Outdated
TWINE_PASSWORD: ${{ secrets.TEST_PYPI_PASSWORD }} | ||
run: | | ||
python3 -m pip install twine | ||
python3 -m twine upload --repository testpypi dist/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could add this, just to provide more info should it ever break
python3 -m twine upload --repository testpypi dist/* | |
python3 -m twine upload --repository testpypi dist/* --verbose |
This should be more robust than just using git e.g. if two PRs are worked on simultaneously.
Updated to use latest version on PyPI rather than by getting number of git commits. Should be more stable if multiple PRs are open. |
It's not clear to me why the publish still fails - have the secrets in this repo been set up correctly? |
Ah yes I see what you are saying, the github actions is reporting the pypi username password is not correct. This needs access to the repository settings to check |
Sorry to be a pain is there any chance this PR can be redirect to the new repository https://github.com/open-radiation-sources/parametric-plasma-source |
This PR allows the plasma source to be installed without requiring a local build. It makes use of manylinux (as defined under pep 513) to create the build on a container that supports forward compatibility. In particular the Python install used is as provided by the container, not the GitHub version. The Python module
auditwheel
is used to ensure that the manylinux wheel is created successfully. This should be compatible with any CentOS 7 (or equivalent) and higher Linux operating system, by using the manylinux2014 standard.