Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove download docker client functionality #633

Merged
merged 2 commits into from
Oct 13, 2024

Conversation

LewisGaul
Copy link
Collaborator

Removes dependency on typer/tqdm/requests by dropping support for downloading the docker client.

Initially agreed at #512 (comment), this has been deprecated since #577 (v0.71.0 released in April 2024), and agreed we can now remove at #552 (comment). If accepted, this supercedes #513 as a resolution to #512. We will also be able to close #575.

@LewisGaul LewisGaul linked an issue Sep 9, 2024 that may be closed by this pull request
@LewisGaul LewisGaul marked this pull request as ready for review September 9, 2024 23:59
@LewisGaul
Copy link
Collaborator Author

@gabrieldemarmiesse are we ready to merge this do you think?

Copy link
Owner

@gabrieldemarmiesse gabrieldemarmiesse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm deeply sorry for the late reply there. Thanks for the PR and the description. I believe we should have a section of the docs dedicated to installing the docker cli without the engine since people whose code won't work anymore will look for a solution and might not be aware that they need an apt-get or similar to replace the "download-cli". If you wish, that can be in another PR, but we should write that up before making a release.

@LewisGaul
Copy link
Collaborator Author

Can we just link to https://docs.docker.com/engine/install/ and https://podman.io/docs/installation? Where in the docs were you thinking this should go?

@gabrieldemarmiesse
Copy link
Owner

gabrieldemarmiesse commented Sep 30, 2024

We should definitely link those.
I think the instructions should go to https://gabrieldemarmiesse.github.io/python-on-whales/docker_client/

I'm not sure linking is enough, we should additionally tell users that they don't need the engine and containerd and don't need to use systemd or systemctl to create the docker service.

The docker docs are not great if you just want to install the client so we should fill this gap. We just need to tell users what steps are not needed.

We can also add a link to this in our README, just after the pip install.

Copy link
Owner

@gabrieldemarmiesse gabrieldemarmiesse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks really good, thanks! We'll pin an issue and display it loudly in the release notes, that should help with visibility. Thanks for everything!

@gabrieldemarmiesse gabrieldemarmiesse merged commit 2e08c98 into gabrieldemarmiesse:master Oct 13, 2024
21 checks passed
@LewisGaul
Copy link
Collaborator Author

Oh, there are some doc updates I was still working on sorry, I'll have to raise in a separate PR

@gabrieldemarmiesse
Copy link
Owner

Something fancy could also be to re-enable CLI entrypoint (without typer) just to print the message that CLI isn't supported by python-on-whales anymore, so that people using it don't just get the "command not found" error. But that's slightly more work. What do you think?

@LewisGaul
Copy link
Collaborator Author

Hm could do, unsure how many users would actually be relying on it...

Here's the follow-up PR: #639

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cache-installed docker doesn't include the compose plugin Make typer/tqdm/requests optional dependencies?
2 participants