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

feat(python): Migrate build to poetry #2024

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

theelderbeever
Copy link

Totally respect just ignoring this PR but I ran into the same problem as noted in #1979 and quickly whipped together a poetry project which appears to address the dependency problem. Feel free to consider it.

closes #1979

@manast
Copy link
Contributor

manast commented Jun 28, 2023

Ok, so poetry is an alternative to pip. In order to use poetry, wouldn't we also need to update github's actions to make a release in poetrie's registry or how does it work in practice?

@manast
Copy link
Contributor

manast commented Jun 28, 2023

@manast
Copy link
Contributor

manast commented Jun 28, 2023

In any case, this seems like an alternative to pip, so not really resolving the issue #1979 which is for pip users.

@theelderbeever
Copy link
Author

theelderbeever commented Jun 28, 2023

@manast A few responses to your questions. Appreciate you giving the PR the time of day.

  1. Yes it is an alternative to pip but also relies on it. It is practically a more powerful dependency resolver with (imho) a more friendly build setup than setup.py provides.
  2. Its still builds a tarball with the poetry build --publish command and publishes to pypi (or the test registry) just as easily without a twine dependency. It doesn't prevent pip users from installing the library. Just changes the dev experience. You can still generate a requirements.txt from the pyproject.toml if you so choose. But with poetry you only provide the dev and release dependencies and then the resolver handles the rest.
  3. Yes, the CI would need updated. I use poetry in my workflows so I can roll those updates into this PR for review if there is further interest in using it.
  4. As for solving [Bug]: ModuleNotFoundError: No module named 'redis' #1979... It doesn't directly solve it but through a (imho) better dependency management/build system it better guarantees the required libraries.

@manast
Copy link
Contributor

manast commented Jun 28, 2023

Ok, I see. So it may actually solve the issue, we should use poetry to publish to pypi instead of twine then.

@theelderbeever
Copy link
Author

Yeah if you opt to use poetry for the packaging/dependencies then its trivial to use for publishing to pypi as well. Just let me know and I can expand the scope of the PR to add poetry into the CI if you like.

[build-system]
requires = ["poetry-core"]
build-backend = "poetry.core.masonry.api"

Copy link
Collaborator

Choose a reason for hiding this comment

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

what about adding semantic-release section here too? https://python-semantic-release.readthedocs.io/en/latest/ to replace setup.cfg config

Copy link
Author

Choose a reason for hiding this comment

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

Poetry manages that too via the pyproject.toml and you can bump the version with poetry version <major | minor | prerelease | ...>. Then I typically do the following to make __version__ always match what is in the pyproject.toml

# bullmq/__init__.py
import importlib.metadata

__version__ = importlib.metadata.version("bullmq")

@theelderbeever
Copy link
Author

theelderbeever commented Jun 28, 2023

Also as a sidenote... since this lib is using async/await I believe the classifiers are incorrect. async/await didn't arrive until version 3.5 I don't think.... Same with type annotations.

        'Programming Language :: Python :: 2',
        'Programming Language :: Python :: 2.7',
        'Programming Language :: Python :: 3',
        'Programming Language :: Python :: 3.4',
        'Programming Language :: Python :: 3.5',

And if I was being overly opinionated I would maybe even limit support to >=3.9 or even 3.10... Not sure what level of community adoption is out there with this library so far.

@vraj-fyno
Copy link

@theelderbeever I had faced the same issue in the slim docker image which was running an older python. The solution I tried was to upgrade the python image.

Limiting the library too 3.10 sounds like a good solution for now?

@theelderbeever
Copy link
Author

@vraj-fyno I would say >=3.10 but yes. IMHO at this point in python's versioning I struggle to see any new projects starting that wouldn't be using 3.10 or 3.11. This also opens the opportunity of using the newer match case and friendlier type annotation syntax.

On another opensource project that I am involved in that was started early last year we made the decision to not support below 3.10 as well.

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.

[Bug]: ModuleNotFoundError: No module named 'redis'
4 participants