Add Transmission get_torrents service and codeowner #159211
Conversation
|
Hey there @engrbm87, @JPHutchins, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
| "active": "Active", | ||
| "all": "All", | ||
| "completed": "Completed (Seeding)", | ||
| "paused": "Paused", | ||
| "started": "Started (Downloading)" |
There was a problem hiding this comment.
So I am a Qbittorrent user, but you can also have torrents where you stop seeding, but don't delete it, that would turn Completed, is that something here as well?
There was a problem hiding this comment.
What does the transmission UI call it?
There was a problem hiding this comment.
I took this from the existing sensor code so have not checked on that scenario. I'll have a look and see if this was correct.
MODES: dict[str, list[str] | None] = {
"started_torrents": ["downloading"],
"completed_torrents": ["seeding"],
"paused_torrents": ["stopped"],
"active_torrents": [
"seeding",
"downloading",
],
"total_torrents": None,
}
There was a problem hiding this comment.
Completed but not seeding torrents currently go into Paused torrents
The current sensors do not total in the same categories as the latest Transmission Flood UI.
Whether historically they did match I'm not sure.
I would prefer to keep the service the same as the sensors, so I have removed the misleading explanations in brackets.
Changing the sensor total methods would introduce a potential breaking change to automations.
There was a problem hiding this comment.
I would expect the 2 also to show in the seeding category lol
There was a problem hiding this comment.
The 2 here are completed but not seeding, hence stopped/paused depending on transmission/integration terminology.
The totals have always been confusing.
There was a problem hiding this comment.
I've compared two of the UI's (there's 4 at least) and they all seem to take a different view on how to total things, the API doesn't give totals and it's left to UI implementation to decide what's right, some UI's give more granular totals than others.
This integration is consistent between the sensors and the new action, I don't think there's a right/wrong way to implement this so think this is resolved.
There was a problem hiding this comment.
Pull request overview
This PR adds a new get_torrents service to the Transmission integration and adds @andrew-codechimp as a codeowner. The service allows users to retrieve current torrents with various filtering options (all, active, started, completed, paused). The changes also refactor torrent filtering logic into a shared helper module.
- Implements a new
get_torrentsservice with filter options - Refactors torrent filtering logic into reusable helper functions
- Updates sensor entities to use the new shared filtering helpers
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/components/transmission/test_services.py | Adds comprehensive tests for the new get_torrents service with various filter modes |
| tests/components/transmission/conftest.py | Updates mock_torrent fixture to include additional fields required for new service |
| homeassistant/components/transmission/strings.json | Adds UI strings for torrent filter options and get_torrents service description |
| homeassistant/components/transmission/services.yaml | Defines service schema for get_torrents with filter selector |
| homeassistant/components/transmission/services.py | Implements get_torrents service logic and registers it with Home Assistant |
| homeassistant/components/transmission/sensor.py | Refactors sensor entities to use new shared filter_torrents helper function |
| homeassistant/components/transmission/manifest.json | Adds @andrew-codechimp to codeowners list |
| homeassistant/components/transmission/icons.json | Adds icon for get_torrents service |
| homeassistant/components/transmission/helpers.py | New module containing shared helper functions for filtering and formatting torrents |
| homeassistant/components/transmission/const.py | Adds constants for new service and moves FILTER_MODES to shared location |
| CODEOWNERS | Updates Transmission component codeowner entries |
| def format_torrent(torrent: Torrent) -> dict[str, Any]: | ||
| """Format a single torrent.""" | ||
| value: dict[str, Any] = {} | ||
|
|
||
| value["id"] = torrent.id | ||
| value["name"] = torrent.name | ||
| value["status"] = torrent.status.value | ||
| value["percent_done"] = f"{torrent.percent_done * 100:.2f}%" | ||
| value["ratio"] = f"{torrent.ratio:.2f}" | ||
| value["eta"] = str(torrent.eta) if torrent.eta else None | ||
| value["added_date"] = torrent.added_date.isoformat() | ||
| value["done_date"] = torrent.done_date.isoformat() if torrent.done_date else None | ||
| value["download_dir"] = torrent.download_dir | ||
| value["labels"] = torrent.labels | ||
|
|
||
| return value |
There was a problem hiding this comment.
This function lacks a docstring explaining the purpose of formatting, the structure of the returned dictionary, or the meaning of fields like 'status.value'. Consider adding a detailed docstring that describes the returned data structure and field types.
There was a problem hiding this comment.
I'm not sure a docstring would add much.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2 participants
Proposed change
Add myself as codeowner to support new functionality.
Implement helper function to filter torrents for use in new service and also sensor entities.
Add a new get_torrents service with filters for types of torrent.
Modify sensors to use new filter function, consolidating filtering.
Add tests for new service.
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: