python-on-whales icon indicating copy to clipboard operation
python-on-whales copied to clipboard

Make typer/tqdm/requests optional dependencies?

Open DanCardin opened this issue 6 months ago • 13 comments

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?

DanCardin avatar Dec 11 '23 15:12 DanCardin

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?

gabrieldemarmiesse avatar Dec 11 '23 16:12 gabrieldemarmiesse

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).

DanCardin avatar Dec 11 '23 16:12 DanCardin

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.

gabrieldemarmiesse avatar Dec 11 '23 16:12 gabrieldemarmiesse

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 :)

LewisGaul avatar Dec 17 '23 21:12 LewisGaul

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 :)

gabrieldemarmiesse avatar Dec 17 '23 22:12 gabrieldemarmiesse

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.

gabrieldemarmiesse avatar Dec 29 '23 20:12 gabrieldemarmiesse

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

LewisGaul avatar Jan 27 '24 17:01 LewisGaul

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?

LewisGaul avatar Jan 27 '24 17:01 LewisGaul

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.

gabrieldemarmiesse avatar Jan 27 '24 17:01 gabrieldemarmiesse

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 :)

gabrieldemarmiesse avatar Jan 27 '24 17:01 gabrieldemarmiesse

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?

LewisGaul avatar Jan 27 '24 17:01 LewisGaul

Yes indeed that's a good idea, I'd be happy to have such a PR

gabrieldemarmiesse avatar Jan 27 '24 17:01 gabrieldemarmiesse

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 :)

gabrieldemarmiesse avatar Feb 23 '24 18:02 gabrieldemarmiesse