test: verify python package distribution build when running make test #65
Conversation
.github/workflows/test.yml
Outdated
| - name: Verify build | ||
| run: | | ||
| pip install --upgrade twine | ||
| python -m build |
There was a problem hiding this comment.
Let's add a make command as build-pythonpackage from tutor core (python -m build --sdist) and use that here.
There was a problem hiding this comment.
I have updated it to reflect the same way we are using it in tutor-core i.e, using make commands and adding the command as part of the make test command which is already being run in the ci.
.github/workflows/test.yml
Outdated
| run: | | ||
| pip install --upgrade twine | ||
| python -m build | ||
| twine check dist/*.tar.gz |
There was a problem hiding this comment.
To be consistent with gitlab flow, lets use similar pattern as twine check dist/$(echo $TUTOR_PYPI_PACKAGE | sed s/-/_/g)-*.tar.gz
There was a problem hiding this comment.
I have updated it to reflect the same way we are using in tutor-core.
Pushing to pypi would occasionally fail because of breaking distribution build. We verify the build in the github CI now so that we can mitigate those errors later on.
Danyal-Faheem
force-pushed
the
danyal/twine-build-ci
branch
from
8ed124d to
bf6ee99
Compare
|
@DawoudSheraz The ci here is failing because the twine version needs to be >6 for this to work with hatch. However, the twine version in tutor v19.0.2 is constrained at 5.1.1. This constraint was upgraded to 6.1.0 with this PR but that commit has not been made a part of any release yet. Adding a constraint in cairn on twine causes a version conflict between tutor and cairn so that is not a solution here. What do you think we should do here? Should we release a new version of tutor to incorporate the pyproject.toml migration and the subsequent requirements update? |
Danyal-Faheem
changed the title
|
My 2 cents: if merging in release is a headache, I think it's fine to add this check to the main branch only. It will get merged after Teak. |
I am ok with releasing a new version. We will be releasing one in two weeks (to incorporate sumac.3), but we can do one today. |
|
@DawoudSheraz I think we can wait till the next tutor release to merge as this PR isn't introducing a necessary fix. |
This is a recurring issue that we usually face whenever a new release is created and didn't show up in the existing CI.