libkeepass / pykeepass

Python library to interact with keepass databases (supports KDBX3 and KDBX4)
https://pypi.org/project/pykeepass/
GNU General Public License v3.0
411 stars 96 forks source link

Poetry migration #370

Closed Ovsyanka closed 4 months ago

Ovsyanka commented 9 months ago

There is inconsistency in dependency management in the project. Some processes use setup.py with almost no version restrictions and some use requirements.txt with exact versions (that is too tight).

Workflow to build the package relies on direct invocation of setup.py, and that is deprecated.

In that PR I address that two issues by replacing setup.py + setuptools by pyproject.toml + poetry.

Details/explanations of the changes

Required python wasn't be specified so I took it from .github/workflows/ci.yaml (meaning 3.6+)

Dependency versions I took from the requirements.txt, but replaced == for ^ because it should be safe enough and not restrict user to outdated versions of the dependencies. Exact dependencies versions proven to work should be managed by poetry.lock file, that is created/updated on changing (adding, removing, updating) dependencies. The only version restriction in setup.py was "pycryptodomex=>3.6.2" and I replaced it with "pycryptodomex^3.6.2" because I think it is safer.

I removed __version__ from the init.py because it would require to use some library like importlib.metadata (available from python 3.8) to get version from the package metadata and there could be different options depending on the python version. As I understand, the library by itself doesn't need that functionality and if someone who uses the library wants to get it's version he can use such a library instead of pykeepass.__version__. As it was used in the make pypi command, I made it upload all new releases to the PyPI instead of just current one. Probably it will be replaced by poetry publish in the future.

Current (1.7.1) version of poetry doesn't support python < 3.8, and poetry 1.1.13 is the last version with python 3.6 support. So in the github CI I use poetry 1.1.13 for both python 3.6 and 3.7, so It will be fine to use it for use the library, but for development current version of poetry expected.

Evidlo commented 9 months ago

Why should running tests depend on Poetry?

Ovsyanka commented 9 months ago

Why should running tests depend on Poetry?

Because to run tests we need to install package dependencies and to install dependencies we need poetry. Before there was requirements.txt, that held dependencies and pip was used directly, now there is poetry.lock instead.

upd: I think I misread your question at first. So, I think you are talking not about why do we need poetry installed, but why do we run tests using poetry run command, right?

This command run it's argument in activated virtual environment, where all the dependencies installed. (see https://python-poetry.org/docs/basic-usage/#using-poetry-run) We are not installing packages globally, like it was before, but in the .venv directory.

Evidlo commented 4 months ago

I've grown a bit uncertain about poetry over the last few months, particularly in regards to how it seems to often break pip editable-installs and makes supporting older versions of Python more complicated.

Sorry for drawing this out for so long. I see that you put in significant effort on this PR and even joined the chatroom to ask questions, so I feel bad about closing this. Hopefully this doesn't discourage you from contributing to this project or other FOSS projects like it in the future.