Open DanCardin opened 9 months ago
I would be ok to make typer optional, but for tqdm, it may be needed if the user doesn't have the CLI locally because it will be downloaded automatically. So I'm afraid it can't be optional :(
Out of curiosity, why are you concerned about the number of dependencies?
I realize that the code that utilizes tqdm is invoked during normal library operations also. but from the perspective of someone importing this library (and running it rather than invoking the CLI), having a tqdm progress bar in my logs is kind of actively the opposite of what I'd want. Although honestly, the automatic downloading of the docker cli itself, is even functionality that i'll never encounter the environments in which I'm using python-on-whales.
Out of curiosity, why are you concerned about the number of dependencies?
It's perhaps partly a purity thing. I dont want to have to install typer/tqdm when I know that they'll never be used and the functionality they provide is completely optional.
More generally, I do think it has positive downstream effects. You may (or may not) be judicious about the versions you depend upon, but adding typer as a dependency just increases the possibility that I or one of my dependencies might depend on a different version that you and cause a conflict (again, for a piece of functionality I wont use).
if you don't want to have a progress bar in the logs, you just have to make sure the docker CLI is installed before running python-on-whales. The auto-download is just here for quick bootstrapping on a new system.
I would like to be able to vendor tqdm, but sadly, vendoring is not supported well in python, so it's not something I can do. I'll accept a PR to make typer optional though.
For what it's worth, I'm in favour of reducing dependencies, where making them optional is a reasonable approach as long as it doesn't diminish the user experience. The problem is doing that without breaking backwards compatibility, as raised in the linked PR.
To be honest, I'm not a massive fan of python-on-whales providing the functionality to fetch a docker binary, let alone auto-downloading it if it's not found, it feels like this should be a prerequisite responsibility of the user to set things up as they wish (docker, podman, ...). What if there's a security patch, python-on-whales will simply never update the downloaded client, whereas a version obtained via a standard package manager is more likely to be.
I think if this behaviour really was desirable then it could make sense to expect the user to opt in to it (rather than having it on by default), e.g. by requiring the extra be specified or splitting into a separate package.
But I think the bottom line is that changing this will involve a (minor and acceptable, in my opinion) backwards compatibility break. That's a judgement call for @gabrieldemarmiesse :)
This is indeed a good insight. The value provided by the auto-download is for total beginners who don't have any knowledge of docker/podman. That's an easy setup. But I agree about the security issue. That's a big concern. Breaking backward compatibility is a big deal, even if it's small, because that might require people to rewrite their pipeline (if we choose to not provide the docker cli). Long story short, I'll think about it, and try to find the least desruptive path toward less dependencies and less security issues, without sacrifying ease of use, that will likely imply writing really good error messages. I just need some time :)
I thought about it and I believe we should get rid completely of the download and CLI commands of Python-on-whale. As such tqdm and typer won't be needed anymore.
About the timing, when we do our first major release, 1.0, we'll do it. I'll post a bit later about what are the requirements for a 1.0 of Python-on-whales. A commitment on backward compatibility is of course on the menu.
The requests
package can be added to the list alongside tqdm
and typer
of those that are only needed for downloading binaries. The list of direct dependencies could be reduced from 5 to 2 (others being pydantic
and typing-extensions
), and the total dependencies down from 12 to 4.
The dependency tree can be seen by running pip-compile
(from the pip-tools
package):
annotated-types==0.6.0
# via pydantic
certifi==2023.11.17
# via requests
charset-normalizer==3.3.2
# via requests
click==8.1.7
# via typer
idna==3.6
# via requests
pydantic==2.5.3
# via python-on-whales (pyproject.toml)
pydantic-core==2.14.6
# via pydantic
requests==2.31.0
# via python-on-whales (pyproject.toml)
tqdm==4.66.1
# via python-on-whales (pyproject.toml)
typer==0.9.0
# via python-on-whales (pyproject.toml)
typing-extensions==4.9.0
# via
# pydantic
# pydantic-core
# python-on-whales (pyproject.toml)
# typer
urllib3==2.1.0
# via requests
I thought about it and I believe we should get rid completely of the download and CLI commands of Python-on-whale.
@gabrieldemarmiesse do you have a suggestion for how to go about doing this, in terms of backwards compatibility?
We'll have breaking changes when doing the v1.0. So that seems like the right time to get rid of the auto-download and CLI commands.
The requests package can be added to the list alongside tqdm and typer of those that are only needed for downloading binaries. The list of direct dependencies could be reduced from 5 to 2 (others being pydantic and typing-extensions), and the total dependencies down from 12 to 4.
That's a good analysis and a good first step :)
We'll have breaking changes when doing the v1.0. So that seems like the right time to get rid of the auto-download and CLI commands.
Okay, would it be worth adding a deprecation warning in, the sooner the better?
Yes indeed that's a good idea, I'd be happy to have such a PR
Hi awesome people, I finally made the issue to describe what the road to 1.0 looks like: #556 , and this issue is on the table :)
It looks like typer
has added a few dependencies recently, giving PoW even more indirect dependencies:
> python3 -V
Python 3.11.3
> pipdeptree -p python-on-whales
python-on-whales==0.71.0
▒▒▒ pydantic [required: >=1.9,<3,!=2.0.*, installed: 2.7.2]
▒ ▒▒▒ annotated-types [required: >=0.4.0, installed: 0.7.0]
▒ ▒▒▒ pydantic_core [required: ==2.18.3, installed: 2.18.3]
▒ ▒ ▒▒▒ typing_extensions [required: >=4.6.0,!=4.7.0, installed: 4.12.0]
▒ ▒▒▒ typing_extensions [required: >=4.6.1, installed: 4.12.0]
▒▒▒ requests [required: Any, installed: 2.32.3]
▒ ▒▒▒ certifi [required: >=2017.4.17, installed: 2024.2.2]
▒ ▒▒▒ charset-normalizer [required: >=2,<4, installed: 3.3.2]
▒ ▒▒▒ idna [required: >=2.5,<4, installed: 3.7]
▒ ▒▒▒ urllib3 [required: >=1.21.1,<3, installed: 2.2.1]
▒▒▒ tqdm [required: Any, installed: 4.66.4]
▒▒▒ typer [required: >=0.4.1, installed: 0.12.3]
▒ ▒▒▒ click [required: >=8.0.0, installed: 8.1.7]
▒ ▒▒▒ rich [required: >=10.11.0, installed: 13.7.1]
▒ ▒ ▒▒▒ markdown-it-py [required: >=2.2.0, installed: 3.0.0]
▒ ▒ ▒ ▒▒▒ mdurl [required: ~=0.1, installed: 0.1.2]
▒ ▒ ▒▒▒ Pygments [required: >=2.13.0,<3.0.0, installed: 2.18.0]
▒ ▒▒▒ shellingham [required: >=1.3.0, installed: 1.5.4]
▒ ▒▒▒ typing_extensions [required: >=3.7.4.3, installed: 4.12.0]
▒▒▒ typing_extensions [required: Any, installed: 4.12.0]
@gabrieldemarmiesse how would you suggest we could go about making this change, is it something to stick on a v1.0 branch or is it a breakage that would be acceptable before the v1.0 release? Note that a deprecation warning was added under https://github.com/gabrieldemarmiesse/python-on-whales/pull/577, although that was only released in the most recent release (v0.71.0).
I think we'll have to wait a bit more to give time to users to change. If the dependencies of typer are really an issue, they can downgrade typer temporarily
As a user of python-on-whales in its capacity as a library (thanks for the library by the way!) I noticed the dependency on typer/tqdm and thought it looked weird.
After looking briefly, it seems to me like at least typer could straightforwardly be shifted to a
cli
extra and definitely be completely optional.tqdm seems to be used a bit more deeply, but would also seem to me to only be a relevant dependency for someone using the CLI. With a bit more work, I could imagine it too being made optional.
Would you accept a PR doing so?