Migrate YouTube channels to config subentries - #181438
Draft
Hugo1380 wants to merge 9 commits into
Draft
Conversation
home-assistant
Bot
added
bugfix
cla-signed
has-tests
integration: youtube
small-pr
Contributor
|
Hey there @joostlek, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Device cleanup currently uses API response data rather than configured channels, potentially deleting valid devices.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Fixes cleanup of YouTube channel devices after channels are removed from integration options.
Changes:
- Corrects prefixed channel identifier matching.
- Adds regression coverage for device removal on reload.
- Cleanup must use configured channels rather than coordinator data.
File summaries
| File | Review |
|---|---|
tests/components/youtube/test_init.py |
Test should clear the configured channel option before reload. |
homeassistant/components/youtube/__init__.py |
Deletion should compare devices against entry.options[CONF_CHANNELS]. |
Review details
Suppressed comments (1)
tests/components/youtube/test_init.py:161
- Update the entry options before reloading. This mock leaves
CONF_CHANNELSunchanged and only makes the API return no data, so the test does not exercise the advertised channel-removal-from-options path.
with patch(
"homeassistant.components.youtube.api.AsyncConfigEntryAuth.get_resource",
return_value=MockYouTube(hass, channel_fixture="get_no_channel.json"),
):
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Hugo1380
force-pushed
the
youtube-fix-delete-devices
branch
from
September 6, 2026 10:37
1f1e796 to
ac81635
Compare
joostlek
requested changes
Sep 6, 2026
joostlek
left a comment
joostlek
left a comment
Member
There was a problem hiding this comment.
I think it makes sense to migrate to subconfig entries
Contributor
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Each tracked channel is now a config subentry of the OAuth entry instead of a list in the entry options. Removing a subentry cleans up its device and entities automatically, which makes the manual delete_devices() cleanup obsolete. - One coordinator per channel subentry - Subentry flow to add channels, options flow removed - Migration from options-based entries (v1) to subentries (v2)
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate issues affect entity registration, migration cleanup, reload behavior, and API error handling.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (4)
homeassistant/components/youtube/init.py:103
- Preserve account-scoped entity unique IDs or explicitly resolve existing duplicates during migration. Version 1 allowed two authenticated accounts to track the same subscribed channel because unique IDs included
entry_id; migrating both to{channel_id}_{sensor_key}makes the secondasync_update_entityraise “unique id is already in use,” preventing that entry from migrating.
entity_registry.async_update_entity(
entity_entry.entity_id,
new_unique_id=entity_entry.unique_id.removeprefix(prefix),
config_subentry_id=subentry.subentry_id,
homeassistant/components/youtube/config_flow.py:302
- Treat only channel subentries as already tracked. A config entry's
unique_ididentifies the authenticated account's own channel even when the user did not select it initially, so adding it here filters that valid channel from the form and makes_async_create_entryabort if the user tries to add it later.
for entry in self.hass.config_entries.async_entries(DOMAIN):
if entry.unique_id:
configured.add(entry.unique_id)
homeassistant/components/youtube/config_flow.py:334
- Abort instead of creating a subentry when the selected channel can no longer be fetched. A subscription can disappear between rendering and submitting the form; the fallback currently saves that invalid channel, after which the automatic reload raises
UpdateFailedand puts the whole YouTube entry into setup retry.
title = channel_id
if channels and channels[0].snippet is not None:
title = channels[0].snippet.title
return self.async_create_entry(
title=title, data={CONF_CHANNEL_ID: channel_id}, unique_id=channel_id
homeassistant/components/youtube/strings.json:35
- Use channel-specific abort messages for the subentry flow. “Account already configured” misidentifies a duplicate channel, and “You need to be subscribed” is shown only after available channels were filtered because they are already tracked, so both messages direct users to the wrong remedy.
"already_configured": "[%key:common::config_flow::abort::already_configured_account%]",
"no_subscriptions": "[%key:component::youtube::config::abort::no_subscriptions%]",
- Files reviewed: 13/13 changed files
- Comments generated: 5
- Review effort level: Balanced
- Keep the entry id prefix in entity unique ids so two accounts can still track the same channel - Only filter channels already tracked by the same entry, allowing the account's own channel to be added later - Clean up devices and entities left behind by untracked channels during migration instead of leaving them attached forever - Create the initial subentries via async_create_entry so the entry is set up once instead of reloading once per selected channel - Fetch playlist items inside the coordinator error boundary - Abort the subentry flow when the selected channel is not available
Hugo1380
changed the title
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Parent setup must remain loaded when an individual channel is missing.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 13/13 changed files
- Comments generated: 1
- Review effort level: Balanced
A channel missing from the API raised ConfigEntryNotReady from its coordinator's first refresh, which put the whole config entry in setup retry and made every healthy channel unavailable too. The per-subentry failure is now contained: the failed coordinator is kept so its entities are created as unavailable while the other channels set up normally. The device name comes from the subentry title, which is always available, and diagnostics report a missing channel as None.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Four moderate issues remain in OAuth refresh, reload handling, title recovery, and diagnostics caching.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
homeassistant/components/youtube/config_flow.py:253
- Refresh the config entry's OAuth session before using its access token. This subentry flow reads the stored token directly here and again in
_async_create_entry; when polling is disabled (or every entity is disabled), no coordinator renews an expired token, so a valid refresh token cannot be used to add a channel and the flow aborts with an access/unknown error. UseOAuth2Session/AsyncConfigEntryAuth.check_and_refresh_token()for both API clients.
- Files reviewed: 13/13 changed files
- Comments generated: 3
- Review effort level: Balanced
…evices # Conflicts: # homeassistant/components/youtube/__init__.py # tests/components/youtube/test_init.py
- Only reload the entry when channel subentries are added or removed: token refreshes persist new entry data and would otherwise cause a full unload/reload on every renewal - Take the migrated subentry title from the existing device name so it survives even if the channel can no longer be fetched from the API - Type the coordinator data as optional and let diagnostics report a channel without data as None instead of discarding cached data after a transient refresh failure
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Token refresh and batched metadata fetching must be addressed, and the orphan cleanup assertion corrected.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
homeassistant/components/youtube/config_flow.py:306
- Revalidate the OAuth token again before fetching the selected channel. The selection form can remain open until the token expires, and this second raw-token authentication then aborts submission even if channel listing originally used a valid token; use the parent entry's
OAuth2Sessionfor this request too.
youtube = YouTube(session=async_get_clientsession(self.hass))
await youtube.set_user_authentication(
config_entry.data[CONF_TOKEN][CONF_ACCESS_TOKEN], [AuthScope.READ_ONLY]
tests/components/youtube/test_init.py:246
- Assert removal using the orphan device's ID. The orphan was created with the prefixed identifier, so looking up the raw identifier returns
Noneeven if migration leaves the original device behind and does not actually test device cleanup.
device_registry.async_get_device_by_identifier(
(DOMAIN, LINUS_CHANNEL_ID), entry.entry_id
- Files reviewed: 13/13 changed files
- Comments generated: 2
- Review effort level: Balanced
The codecov patch/required check expects full coverage of the changed config flow and diagnostics lines. Remove the dead aborts in the initial channels step (already guaranteed by the OAuth step) and the redundant already-configured check (raised by the config entries manager), and add tests for the subentry flow API error paths. The migration test now uses a stale device name so the title refresh path is covered too.
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The subentry flow may use expired OAuth credentials when no coordinator remains, preventing channel addition.
Review details
Suppressed comments (1)
homeassistant/components/youtube/config_flow.py:246
- Refresh the OAuth token before using it in the subentry flow. After the last channel is removed there is no coordinator left to call
async_ensure_token_valid(), so once the stored access token expires this request uses stale credentials and adding a channel aborts asunknown; obtain the token through anOAuth2Session/AsyncConfigEntryAuthfor both listing and fetching channels.
) = await async_get_channel_options(
self.hass, config_entry.data[CONF_TOKEN][CONF_ACCESS_TOKEN]
- Files reviewed: 13/13 changed files
- Comments generated: 0 new
- Review effort level: Balanced
…y flow - The subentry flow now obtains its YouTube client through an AsyncConfigEntryAuth built on the parent entry's OAuth2 session, so the token is refreshed when needed instead of reusing the stored access token, which could be expired after the last channel was removed and no coordinator renewed it anymore - One account-level coordinator fetches all tracked channels in a single batched get_channels call again; a channel missing from the response is simply absent from the data and only its entities become unavailable, keeping per-subentry availability without N API calls per refresh - The migration test asserts orphan cleanup by device id
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Two moderate exception-handling issues can cause configuration flows to fail unexpectedly.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 2
- Review effort level: Balanced
Wrap the channel listing of the initial channels step in the same exception boundary as the OAuth step so failures abort as access_not_configured or unknown instead of escaping the flow, and move the subentry flow's OAuth client creation inside the existing boundary since refreshing the token can raise too.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The coordinator can exceed YouTube’s 50-ID request limit, making every tracked channel unavailable.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 1
- Review effort level: Balanced
Comment on lines
+85
to
+89
| channel_ids = [ | ||
| subentry.data[CONF_CHANNEL_ID] | ||
| for subentry in self.config_entry.get_subentries_of_type( | ||
| SUBENTRY_TYPE_CHANNEL | ||
| ) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed change
Per the code owner's request, this PR now migrates the YouTube integration to config subentries: each tracked channel becomes a
channelsubentry of the OAuth config entry, instead of a list of channel ids in the entry options.History:
delete_devices()had been a silent no-op since #107907 (prefixed identifiers never matched raw channel ids, and the condition was inverted), so devices and entities of channels removed from the options stayed attached to the config entry forever. Migrating to subentries makes that manual cleanup obsolete: removing a subentry lets Home Assistant clean up its device and entities automatically.What changed:
channelsubentry flow (searchable dropdown of own channels and subscriptions); the initial multi-select step is kept and creates the subentries atomically viaasync_create_entry(subentries=...)options[CONF_CHANNELS], devices moved to their subentry, entities attached to it — and the devices/entities left behind by the old deletion bug are finally cleaned upRegression tests cover the migration (including orphan cleanup), subentry addition/removal and a channel missing from the API.
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: