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

automate pypi deploy #106

Closed
wants to merge 18 commits into from
Closed

automate pypi deploy #106

wants to merge 18 commits into from

Conversation

tedchou12
Copy link

closes #105
Need to add PYPI_TOKEN to secret.

@ojii
Copy link
Contributor

ojii commented Nov 16, 2021

it shouldn't release without tests passing, so this should either be moved to cci or the cci stuff needs to be moved to GHA.

Also, doesn't this need some sort of "is this actually a new version" check beyond just "it's on the master branch"?

.github/workflows/deploy.yml Outdated Show resolved Hide resolved
.github/workflows/deploy.yml Outdated Show resolved Hide resolved

- name: pypi deployments
run: |
poetry build
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this run after or in parallel to the unit tests?

Copy link
Author

Choose a reason for hiding this comment

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

As @ojii suggested, putting to circleci workflow is probably better!

@dimaqq
Copy link
Contributor

dimaqq commented Nov 16, 2021

it shouldn't release without tests passing, so this should either be moved to cci or the cci stuff needs to be moved to GHA.

Also, doesn't this need some sort of "is this actually a new version" check beyond just "it's on the master branch"?

pypi will reject reupload of the same version.
thus, it's kinda :fine:
(not totally fine, because the action needs to silence upload errors)
(fine here because it's one package for all arch's, but would not be fine if we built arch-specific wheels)

I think perhaps the GHA way would be to trigger on release rather than on push?

If we wanted to hack this manually:
Maybe it's possible to examine the last commit diff to see if the version changed?
Or poke pypi and see if the local version is different?

@tedchou12
Copy link
Author

tedchou12 commented Nov 16, 2021

yes, I think moving to CircleCI is better. I made the tests required before deploy.

Also, doesn't this need some sort of "is this actually a new version" check beyond just "it's on the master branch"?

As @dimaqq suggested, the poetry will automatically reject if a same version is uploaded with 400 error, so I guess it will hint the necessity to bump the version.
Is possible to compare the versions before upload, but I guess just doing a overall checksum is not safe while getting the version number is a bit messy.
import toml;pkg_v = toml.load('pyproject.toml')['tool']['poetry']['version'];import luddite;pypi_v=luddite.get_version_pypi('aiodynamo')\nif (pypi_v != pkg_v): print('1')\nelse: print('0');

to determine which branch should trigger the action, is possible to specify with the following filter, sorry, I am not sure about the workflow for this, I will leave this part to you.

filters:
            branches:
              only:
                - master

As for the ${PYPI_TOKEN}, it is needed to be inputted into the environmental variables in circleci.

@dimaqq
Copy link
Contributor

dimaqq commented Jan 25, 2022

This effort needs a new champion!

@ojii
Copy link
Contributor

ojii commented Jan 25, 2022

i've tried to port cci to gha but got disillusioned by act https://github.com/HENNGE/aiodynamo/tree/gha

@dimaqq
Copy link
Contributor

dimaqq commented Jan 25, 2022

Hmm unhashable VersionUnion was a poetry bug python-poetry/poetry#2340
I wonder what poetry version the action you use pulls in?

@dimaqq
Copy link
Contributor

dimaqq commented Jan 25, 2022

My 2c: how about smaller scope: for example, only py3.9, only pytest tests; leave mypy/isort/versions to the maintainers, at least at the start?

@dimaqq
Copy link
Contributor

dimaqq commented Jan 25, 2022

re: poetry failing, it's possible to specify exactly what poetry version to use:

        uses: abatilo/actions-poetry@v2.0.0
        with:
          poetry-version: ${{ matrix.poetry-version }}

(perhaps a constant in this case)

@ojii
Copy link
Contributor

ojii commented Jan 25, 2022

re: poetry failing, it's possible to specify exactly what poetry version to use:

it's supposed to use 1.1.11 https://github.com/abatilo/actions-poetry/blob/7044c9c69e0265717d52471f66033b8d0e2a69ff/action.yml

@ojii
Copy link
Contributor

ojii commented Apr 20, 2022

closing this since it's pretty out of date now due to #122

This should probably be discussed a bit more in #105, specifically with regards to when to publish a new version and how/where version numbers are handled.

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.

Faster release cycle
3 participants