ALSchwalm / transient

A wrapper for QEMU allowing the creation of virtual machines with shared folder, ssh, and disk creation support.
MIT License
104 stars 17 forks source link

Update to Poetry #156

Closed pdietl closed 1 week ago

pdietl commented 2 years ago

The setup.py-style builds a deprecated. PEP 517 introduced the pyproject.toml way of packing Python projects. This PR switches to this new mechanism and uses the Poetry build system. The advantage of this system is that a Rust-inspired lock file is created to lock down exact dependencies and and all but guarantees that future users will be able to build and use the project.

I was inspired to make this change after running into a dependency problem where one package required Click<8.0.0 and another required Click>=8.0.0. The was probably not a problem at some point in the past, but because there was no lock file, a transitive dependency got updated and the conflict was introduced.

pdietl commented 2 years ago

@ALSchwalm does the last GitHub Actions failure looks like a race condition bug in the test to you? (Also, Hi!)

ALSchwalm commented 2 years ago

Hey! Poetry is definitely an improvement and switching to it is probably a good idea. I had some trouble testing this due to some weird keyring expectations from poetry (seemingly relevant to https://github.com/python-poetry/poetry/issues/1917), which for reference can be resolved by setting PYTHON_KEYRING_BACKEND=keyring.backends.null.Keyring, if you don't have a keyring because you keep all your passwords on a post-it note like me. Some questions:

All of the tests are passing locally for me, so I'd presume any issues your hitting are related to the fact that qemu can't be run with hardware acceleration in CI, so it is very slow. The timeouts for things are generous but still can just expire sometimes. I can take a closer look in a bit.

pdietl commented 2 years ago

Oh man the key ring issue is definitely no bueno. Are still ok using Poetry or would you rather we switch to some other build system?

Regarding Python 3.6 support, I am pretty sure we can keep support for it. I'll just have to play around more precisely with dependency versions to find a combination that works. This shouldn't be too difficult.

ALSchwalm commented 2 years ago

It seems like poetry has the momentum behind it (to my knowledge). The devs in that thread seem to indicate that these kinds of behaviors will be fixed over time, so I think I'm ok switching to it as long as a user that just pip installs the wheel from pypi isn't impacted.

pdietl commented 2 years ago

Dropping Python 3.6 seems necessary. Since it is no longer supported, some dependencies are not happy with Python 3,6 anymore.

pdietl commented 2 years ago

@ALSchwalm can you help me fix this type error?

ALSchwalm commented 2 years ago

It appears that type info has been added for lark which has revealed the incorrect type info in the build.py file. Probably not too bad to fix that with some helpers to check the types of the children of ast nodes and ensure things are as expected.

I can probably tackle this in the next few days. Looks like you've resolved everything else though! Thanks for working on this

ALSchwalm commented 2 years ago

Finally got around to this, should be fixed on your branch now