-
-
Notifications
You must be signed in to change notification settings - Fork 36.7k
Portainer add prune unused images #160137
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
Portainer add prune unused images #160137
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds functionality to prune unused Docker images at the endpoint level in the Portainer integration. The feature is exposed through a new button entity for each endpoint, allowing users to manually trigger image pruning operations. The PR also improves existing code by adding translation keys for previously hardcoded button names.
Key changes:
- Adds
PortainerEndpointButtonentity class for endpoint-level button actions - Creates "Prune unused images" button for each Portainer endpoint
- Migrates existing container restart buttons to use translation keys instead of hardcoded names
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
homeassistant/components/portainer/button.py |
Adds endpoint button support with new PortainerEndpointButton class, creates ENDPOINT_BUTTONS tuple with images_prune action, renames BUTTONS to CONTAINER_BUTTONS, and adds translation keys for existing buttons |
homeassistant/components/portainer/strings.json |
Adds translation strings for button entities including images_prune, restart_container, and prune_endpoint |
tests/components/portainer/test_button.py |
Adds comprehensive test coverage for endpoint button functionality including success and error scenarios, renames existing test functions for clarity |
tests/components/portainer/snapshots/test_button.ambr |
Updates snapshots to reflect translation key changes and adds snapshot for new prune_unused_images button entity |
tests/components/portainer/conftest.py |
Adds images_prune mock method to the mock Portainer client |
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Co-authored-by: Copilot <[email protected]>
liudger
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| translation_domain=DOMAIN, | ||
| translation_key="cannot_connect", | ||
| translation_placeholders={"error": repr(err)}, | ||
| translation_placeholders={"error": "See details in logs"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now we'd have the untranslated string "See details in logs" in the UI and still a mix of languages
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see other integrations still having the repr or str method. What do you propose in this scenario to implement?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you propose in this scenario to implement?
depends a bit on my next question below. In this case I'd just remove that entirely tbh
I see other integrations still having the repr or str method.
yes, we do have that in the codebase currently, need to clean that up at some point
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can leave it empty, but can't fully remove it, since it's defined in the strings.json. Creating a new exception string for this, seems a bit overkill?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's our best best tbh, because looking at the string it's now "something happened: ", which might look confusion to the user
|
@zweckj exceptions with no details have been added. |
Breaking change
Proposed change
Add the functionality per endpoint to prune unused images. In the current implementation this is made available via a button on the endpoint entity.
Later on this can be extended to an Action, so users can use this more freely and create their own automations (i.e.: dangling or not, when to prune based on the until paramater made available via pyportainer).
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: