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

Add stream output for pruning #566

Merged
merged 17 commits into from
Mar 22, 2024

Conversation

anesmemisevic
Copy link
Contributor

Resolves #396
Not sure if we need to update this? #499

  • added function overloading for pruning
  • added relevant tests to check the streaming logs/output

Will complete the overloading for other pruning, such as for Docker Image, Network, Buildx, etc. when this one gets approved.

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.

The feature looks good and the tests too. In terms of readability, we'll prefer to put the if statement outside the assert to lower the complexity of the logic and make it more readable. The else True becomes then unnecessary.

After those small fixes, we can merge :) thanks for the PR!

python_on_whales/components/container/cli_wrapper.py Outdated Show resolved Hide resolved
tests/python_on_whales/components/test_container.py Outdated Show resolved Hide resolved
tests/python_on_whales/components/test_container.py Outdated Show resolved Hide resolved
tests/python_on_whales/components/test_container.py Outdated Show resolved Hide resolved
tests/python_on_whales/components/test_container.py Outdated Show resolved Hide resolved
tests/python_on_whales/components/test_container.py Outdated Show resolved Hide resolved
tests/python_on_whales/components/test_container.py Outdated Show resolved Hide resolved
@gabrieldemarmiesse
Copy link
Owner

Will complete the overloading for other pruning, such as for Docker Image, Network, Buildx, etc. when this one gets approved.

Indeed this seems like a good idea, we'll have a consistent api :)

anesmemisevic and others added 7 commits March 21, 2024 16:01
Co-authored-by: Gabriel de Marmiesse <gabrieldemarmiesse@gmail.com>
Co-authored-by: Gabriel de Marmiesse <gabrieldemarmiesse@gmail.com>
Co-authored-by: Gabriel de Marmiesse <gabrieldemarmiesse@gmail.com>
Co-authored-by: Gabriel de Marmiesse <gabrieldemarmiesse@gmail.com>
Co-authored-by: Gabriel de Marmiesse <gabrieldemarmiesse@gmail.com>
@gabrieldemarmiesse gabrieldemarmiesse merged commit 070bf6e into gabrieldemarmiesse:master Mar 22, 2024
35 checks passed
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.

docker.system.prune doesn't output anything
2 participants