HENNGE / aiodynamo

Asynchronous, fast, pythonic DynamoDB Client
https://aiodynamo.readthedocs.io/
Other
73 stars 21 forks source link

automate pypi deploy #106

Closed tedchou12 closed 2 years ago

tedchou12 commented 2 years ago

closes #105 Need to add PYPI_TOKEN to secret.

ojii commented 2 years ago

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"?

dimaqq commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

This effort needs a new champion!

ojii commented 2 years ago

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

dimaqq commented 2 years ago

Hmm unhashable VersionUnion was a poetry bug https://github.com/python-poetry/poetry/issues/2340 I wonder what poetry version the action you use pulls in?

dimaqq commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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.