Migrate from setup.py to pyproject.toml and Poetry #506
Conversation
FuzzyReality
added
breaking-change
FuzzyReality
force-pushed
the
migrate
branch
2 times, most recently
from
ae8725f to
c9dd6c2
Compare
We have decided to move from setup.py to Poetry and pyproject.toml as our python package and dependency management tool. This is mainly done in order to ease the integration of SNYK. SNYK is used for vulnerability scanning of our dependencies.
FuzzyReality
force-pushed
the
migrate
branch
2 times, most recently
from
bd65c61 to
ea3ab4e
Compare
In addition to publishing, this workflow will also update all dependencies in poetry.lock to the newest available versions. This is done on the assumption that the newest versions are "safer" to use than older ones. They may introduce new bugs, but are more likely to remove old ones.
The pyporject.toml and poetry.lock were not included in the file, thus causing the dockerfile to fail its copy of these files.
Changed usage of pip to poetry where relevant. Locked poetry install version to the latest available version, to ensure stability when building.
Add poetry to the pyproject.toml as a dependecy in order to ensure that SNYK monitors poetry as well. Currently, this one must manually be kept in synk with the Dockerfile and GH actions. Depending on how SNYK for docker works, this may or may not be overkill.
Setup.py is no longer needed, as we use pyproject isntead now.
FuzzyReality
force-pushed
the
migrate
branch
from
ea3ab4e to
0b7d052
Compare
|
Progresses equinor/robotics-infrastructure#243, equinor/robotics-infrastructure#225 and equinor/robotics-infrastructure#315 |
FuzzyReality
requested review from
aeshub,
Christdej and
tsundvoll
and removed request for
aeshub and
Christdej
| __version__ = get_distribution(__name__).version | ||
| except DistributionNotFound: | ||
| pass # package is not installed | ||
| __version__ = "1.16.8" |
There was a problem hiding this comment.
What was the reason for changing this file?
There was a problem hiding this comment.
From https://setuptools.pypa.io/en/latest/pkg_resources.html.
Worth noting that the distutils package is removed from the standard library in python 3.12.
As is, this version is updated as part of the publishing workflow here in GH, based on the release tag of the release. As such, I should probably change it to 0.0.0 as a placeholder to make that clearer.
There was a problem hiding this comment.
Could just be removed altogether or not?
| python -m twine upload --verbose --skip-existing -p ${{ secrets.PYPI_TOKEN }} -u __token__ dist/* | ||
| poetry config repositories.publish https://pypi.org/legacy/ | ||
| poetry publish -p ${{ secrets.PYPI_TOKEN }} -u __token__ -r publish --build | ||
| shell: bash |
| # This is an adaptation of https://github.com/code-specialist/pypi-poetry-publish. | ||
| # That action is released under an MIT License, and as such, so is this workflow. |
There was a problem hiding this comment.
Maybe, maybe not. ISAR uses EPLv2, while the one I based this workflow off uses MIT. Don't feel I've changed it sufficiently from the original to justify changing the License. (And I've also been trained to reference my sources ;) )
I'd be happy to discuss this.
| @@ -14,28 +14,32 @@ jobs: | |||
| strategy: | |||
| fail-fast: false | |||
| matrix: | |||
| python-version: ["3.10", "3.11", "3.12"] | |||
| python-version: ["3.10", "3.11", "3.12", "3.x"] # pythonpublish.yml uses "3.x". | |||
There was a problem hiding this comment.
Maybe we should change pythonpublish to only include 10, 11 and 12 then?
There was a problem hiding this comment.
I'm fine with that. Added 3.x as pythonpublish.yml was (and still is) using 3.x. I suggest that we change pythonpublish to use one of the above instead of 3.x.
| @@ -7,18 +7,19 @@ RUN python -m venv --copies $VIRTUAL_ENV | |||
| ENV PATH="$VIRTUAL_ENV/bin:$PATH" | |||
|
|
|||
| RUN python -m pip install --upgrade pip | |||
| RUN pip install poetry==1.7.0 | |||
There was a problem hiding this comment.
Is it necessary to lock the version of this?
There was a problem hiding this comment.
It isn't, although it is recommended to
|
This pull request has automatically been marked as stale as there has been no activity for 30 days. |
|
I suggest we migrate from setup.py to pyproject.toml as proposed by this PR as well, but that we stick to using pip and setuptools for packaging and building. A have made a PR on isar-robot with my suggested solution: equinor/isar-robot#103. Something similar can be done for ISAR if we decide to to that way. |
| - name: Set GitHub Tag as Package Version | ||
| run: | | ||
| sed -i -r 's/__version__ *= *".*"/__version__ = "${{ github.event.release.tag_name }}"/g' ./src/isar/__init__.py | ||
| sed -i '0,/version =.*/s//version = "'"${{ github.event.release.tag_name }}"'"/' ./pyproject.toml | ||
| shell: bash |
There was a problem hiding this comment.
If we stick to setuptools, we can use the SCM tag (github release tag) for setting the version and we avoid having a separate version bump commit.
See example here: https://github.com/equinor/isar-robot/pull/103/files#diff-50c86b7ed8ac2cf95bd48334961bf0530cdc77b5a56f852c5c61b89d735fd711R34-R35
We have decided to move from
setup.pyto poetry andpyproject.tomlas our python package and dependency management tool. This is mainly done in order to ease the integration of SNYK. SNYK is used for vulnerability scanning of our dependencies.As part of this, relevant workflows have also been updated to work with poetry.