Arksine / moonraker

Web API Server for Klipper
https://moonraker.readthedocs.io
GNU General Public License v3.0
1.05k stars 406 forks source link

pdm is used. But not really. #815

Closed kdomanski closed 7 months ago

kdomanski commented 7 months ago

What happened

Hey, I'm trying to contribute to this project, and an attempt to add a dependency sent me down the rabbit hole. Using pdm properly is currently not possible, as the tool reports all kinds of problems when trying to e.g. sync the lockfile or add a new dependency.

A cursory look at the commit history of pyproject.toml and pdm.lock shows that the latter remains basically untouched, which tells me that in reality you're doing dependency management with pip. Moreover the python_version specifiers in pyproject.toml are simply not supported in most pdm operations. See this comment from pdm developer Moonraker's installation docs, as well as install-moonraker.sh also use pip.

With that, I propose either one of the following:

  1. Drop pdm entirely, stick to pip.
  2. Go all-in on pdm:
    • drop Python 3.7 (which is EOL) and add safeties to prevent breakage of existing installations
    • sort out pdm.lock and autoamtically enforce that it matches pyproject.toml
    • rewrite installation scripts/docs

Either way, I'm happy to contribute my time and help do this.

Client

Other

Browser

Other or N/A

How to reproduce

Either one of:

Additional information

No response

Arksine commented 7 months ago

Indeed you are correct, I'm not currently using PDM for dependency management. I'm only using its backend to perform the build.

I did plan to do so, but not yet. While Python 3.7 is EOL, there are still many users on Debian Buster, so I have pushed back my timeline on removing 3.7 support. Currently I plan drop support for Python 3.7 when 3.8 goes EOL so both can be dropped. Time permitting, I'd like to publish a stable release this summer, the one and only supporting Python 3.7/3.8. I do have some items on the agenda I'd like to get done before I do this.

You do raise a good point. Since PDM management isn't working, its probably for the best to remove the lock file and badge. It can always be added at a later time should it be desired.

Arksine commented 7 months ago

Commit e87ab4a8955ca434b7f7c09d3eba443200cb523d made those changes to resolve any future confusion.

kdomanski commented 7 months ago

Thanks, that should save the next person a lot of head scratching.