fix(channels): a channel YouTube no longer returns is not asked for on every view
channels.list simply omits a terminated channel, so nothing was stamped and the channel page spent a quota unit on it every time. The miss is stamped like a fetch; the published_at sentinel that kept it stale is gone.
This commit is contained in:
@@ -190,7 +190,7 @@ def discover_channels(
|
|||||||
quota.attribute(user.id, quota.QuotaAction.CHANNELS_DISCOVER),
|
quota.attribute(user.id, quota.QuotaAction.CHANNELS_DISCOVER),
|
||||||
YouTubeClient(db, user) as yt,
|
YouTubeClient(db, user) as yt,
|
||||||
):
|
):
|
||||||
apply_channel_details(db, yt.get_channels(need))
|
apply_channel_details(db, yt.get_channels(need), need)
|
||||||
db.commit()
|
db.commit()
|
||||||
# Match _discovery_rows' ORDER BY exactly: video-count DESC, then title (NULLs LAST,
|
# Match _discovery_rows' ORDER BY exactly: video-count DESC, then title (NULLs LAST,
|
||||||
# as Postgres orders them — coercing NULL→"" would wrongly float untitled channels up).
|
# as Postgres orders them — coercing NULL→"" would wrongly float untitled channels up).
|
||||||
@@ -296,20 +296,20 @@ def channel_detail(
|
|||||||
"""One channel's "About" detail for the channel page header/About tab, plus this user's
|
"""One channel's "About" detail for the channel page header/About tab, plus this user's
|
||||||
relationship to it (subscribed / explored) and how many of its videos are in the catalog.
|
relationship to it (subscribed / explored) and how many of its videos are in the catalog.
|
||||||
Lazily fills the About metadata (view count, join date, banner, links) the first time it's
|
Lazily fills the About metadata (view count, join date, banner, links) the first time it's
|
||||||
needed — a stub, or a channel last enriched before those fields existed (published_at is a
|
needed (a stub) and refreshes it once it is stale (`DETAILS_MAX_AGE`), which is how a channel nobody subscribes to (an explored one, outside the
|
||||||
reliable sentinel: every real channel has a creation date) — and refreshes it once it is stale
|
|
||||||
(`DETAILS_MAX_AGE`), which is how a channel nobody subscribes to (an explored one, outside the
|
|
||||||
subscription import) gets a banner that still resolves. The enrich uses the API key (1 unit,
|
subscription import) gets a banner that still resolves. The enrich uses the API key (1 unit,
|
||||||
cached after), so it's skipped for the quota-less demo account."""
|
cached after), so it's skipped for the quota-less demo account."""
|
||||||
channel = get_or_404(db, Channel, channel_id, what="channel")
|
channel = get_or_404(db, Channel, channel_id, what="channel")
|
||||||
stale = channel.published_at is None or details_stale(channel, datetime.now(timezone.utc))
|
# No `published_at is None` sentinel: a terminated channel never gets one, so it re-fetched on
|
||||||
|
# every view; rows enriched before that field existed are long past DETAILS_MAX_AGE anyway.
|
||||||
|
stale = details_stale(channel, datetime.now(timezone.utc))
|
||||||
if stale and not user.is_demo:
|
if stale and not user.is_demo:
|
||||||
try:
|
try:
|
||||||
with (
|
with (
|
||||||
quota.attribute(user.id, quota.QuotaAction.CHANNELS_DISCOVER),
|
quota.attribute(user.id, quota.QuotaAction.CHANNELS_DISCOVER),
|
||||||
YouTubeClient(db, user) as yt,
|
YouTubeClient(db, user) as yt,
|
||||||
):
|
):
|
||||||
apply_channel_details(db, yt.get_channels([channel_id]))
|
apply_channel_details(db, yt.get_channels([channel_id]), [channel_id])
|
||||||
db.commit()
|
db.commit()
|
||||||
channel = db.get(Channel, channel_id)
|
channel = db.get(Channel, channel_id)
|
||||||
except YouTubeError as exc:
|
except YouTubeError as exc:
|
||||||
@@ -415,12 +415,10 @@ def explore_channel(
|
|||||||
YouTubeClient(db, user) as yt,
|
YouTubeClient(db, user) as yt,
|
||||||
):
|
):
|
||||||
# Enrich for the About panel + the uploads playlist id (needed to ingest videos).
|
# Enrich for the About panel + the uploads playlist id (needed to ingest videos).
|
||||||
if (
|
# Stale, not "a field is missing": a terminated channel never gets an uploads playlist
|
||||||
channel.details_synced_at is None
|
# or a creation date, and asking again on every explore would spend quota on a miss.
|
||||||
or channel.uploads_playlist_id is None
|
if details_stale(channel, datetime.now(timezone.utc)):
|
||||||
or channel.published_at is None
|
apply_channel_details(db, yt.get_channels([channel_id]), [channel_id])
|
||||||
):
|
|
||||||
apply_channel_details(db, yt.get_channels([channel_id]))
|
|
||||||
db.commit()
|
db.commit()
|
||||||
channel = db.get(Channel, channel_id)
|
channel = db.get(Channel, channel_id)
|
||||||
result = explore_ingest_page(db, yt, channel, page_token)
|
result = explore_ingest_page(db, yt, channel, page_token)
|
||||||
@@ -593,7 +591,7 @@ def subscribe(
|
|||||||
# must not block recording the subscription the user asked for.
|
# must not block recording the subscription the user asked for.
|
||||||
if channel.details_synced_at is None:
|
if channel.details_synced_at is None:
|
||||||
try:
|
try:
|
||||||
apply_channel_details(db, yt.get_channels([channel_id]))
|
apply_channel_details(db, yt.get_channels([channel_id]), [channel_id])
|
||||||
except YouTubeError as exc:
|
except YouTubeError as exc:
|
||||||
log.warning(
|
log.warning(
|
||||||
"subscribe: stub enrich failed: %s",
|
"subscribe: stub enrich failed: %s",
|
||||||
|
|||||||
@@ -130,7 +130,7 @@ def _ingest_candidates(
|
|||||||
try:
|
try:
|
||||||
with quota.attribute(user.id, quota.QuotaAction.CHANNELS_DISCOVER):
|
with quota.attribute(user.id, quota.QuotaAction.CHANNELS_DISCOVER):
|
||||||
details = yt.get_channels(new_channel_ids)
|
details = yt.get_channels(new_channel_ids)
|
||||||
apply_channel_details(db, details)
|
apply_channel_details(db, details, new_channel_ids)
|
||||||
db.commit()
|
db.commit()
|
||||||
except YouTubeError:
|
except YouTubeError:
|
||||||
# The stubs remain and the details can be backfilled later — but until they are, those
|
# The stubs remain and the details can be backfilled later — but until they are, those
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
"""Import the authenticated user's YouTube subscriptions and channel metadata."""
|
"""Import the authenticated user's YouTube subscriptions and channel metadata."""
|
||||||
|
|
||||||
|
from collections.abc import Iterable
|
||||||
from datetime import datetime, timedelta, timezone
|
from datetime import datetime, timedelta, timezone
|
||||||
|
|
||||||
from sqlalchemy import or_, select
|
from sqlalchemy import or_, select
|
||||||
@@ -68,8 +69,16 @@ def details_stale_clause(now: datetime):
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
def apply_channel_details(db: Session, items: list[dict]) -> None:
|
def apply_channel_details(db: Session, items: list[dict], asked: Iterable[str]) -> None:
|
||||||
|
"""Write `channels.list`'s answer onto the rows. `asked` is every id the call asked for: one
|
||||||
|
YouTube did not return (terminated, deleted) is stamped as synced too — otherwise it stays
|
||||||
|
stale forever and every view / import asks for it again, spending quota on a known miss."""
|
||||||
now = datetime.now(timezone.utc)
|
now = datetime.now(timezone.utc)
|
||||||
|
returned = {item["id"] for item in items}
|
||||||
|
for cid in set(asked) - returned:
|
||||||
|
missing = db.get(Channel, cid)
|
||||||
|
if missing is not None:
|
||||||
|
missing.details_synced_at = now
|
||||||
for item in items:
|
for item in items:
|
||||||
channel = db.get(Channel, item["id"])
|
channel = db.get(Channel, item["id"])
|
||||||
if channel is None:
|
if channel is None:
|
||||||
@@ -179,8 +188,9 @@ def import_subscriptions(db: Session, user: User) -> dict:
|
|||||||
)
|
)
|
||||||
detailed = 0
|
detailed = 0
|
||||||
if need_details:
|
if need_details:
|
||||||
items = yt.get_channels([c.id for c in need_details])
|
ids = [c.id for c in need_details]
|
||||||
apply_channel_details(db, items)
|
items = yt.get_channels(ids)
|
||||||
|
apply_channel_details(db, items, ids)
|
||||||
detailed = len(items)
|
detailed = len(items)
|
||||||
db.commit()
|
db.commit()
|
||||||
|
|
||||||
|
|||||||
@@ -28,7 +28,7 @@ def _item(cid: str) -> dict:
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
def _youtube(asked: list[list[str]], subscribed: list[str]):
|
def _youtube(asked: list[list[str]], subscribed: list[str], gone: tuple[str, ...] = ()):
|
||||||
class _Client:
|
class _Client:
|
||||||
def __init__(self, db, user):
|
def __init__(self, db, user):
|
||||||
pass
|
pass
|
||||||
@@ -45,7 +45,8 @@ def _youtube(asked: list[list[str]], subscribed: list[str]):
|
|||||||
|
|
||||||
def get_channels(self, ids):
|
def get_channels(self, ids):
|
||||||
asked.append(sorted(ids))
|
asked.append(sorted(ids))
|
||||||
return [_item(cid) for cid in ids]
|
# A terminated / deleted channel is simply absent from `channels.list`'s answer.
|
||||||
|
return [_item(cid) for cid in ids if cid not in gone]
|
||||||
|
|
||||||
return _Client
|
return _Client
|
||||||
|
|
||||||
@@ -105,3 +106,38 @@ def test_the_channel_page_refreshes_a_stale_channel_only(
|
|||||||
assert asked == ([["UCpage"]] if refreshed else [])
|
assert asked == ([["UCpage"]] if refreshed else [])
|
||||||
suffix = "new" if refreshed else "old"
|
suffix = "new" if refreshed else "old"
|
||||||
assert body["banner_url"] == f"https://yt3.example/UCpage-{suffix}"
|
assert body["banner_url"] == f"https://yt3.example/UCpage-{suffix}"
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_channel_youtube_no_longer_returns_is_not_asked_for_on_every_view(
|
||||||
|
db, make_user, monkeypatch
|
||||||
|
):
|
||||||
|
"""A terminated channel is missing from `channels.list`'s answer, so nothing was stamped and the
|
||||||
|
channel page asked for it again (1 quota unit) on every single view. The miss is stamped like a
|
||||||
|
fetch, so the next ask waits for `DETAILS_MAX_AGE` like any other channel."""
|
||||||
|
user = make_user()
|
||||||
|
db.add(Channel(id="UCgone", title="UCgone"))
|
||||||
|
db.commit()
|
||||||
|
asked: list[list[str]] = []
|
||||||
|
monkeypatch.setattr(channels, "YouTubeClient", _youtube(asked, [], gone=("UCgone",)))
|
||||||
|
|
||||||
|
channels.channel_detail("UCgone", user=user, db=db)
|
||||||
|
channels.channel_detail("UCgone", user=user, db=db)
|
||||||
|
|
||||||
|
assert asked == [["UCgone"]]
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_import_stamps_a_channel_youtube_no_longer_returns(db, make_user, monkeypatch):
|
||||||
|
user = make_user()
|
||||||
|
_channel(db, "UCgone", STALE)
|
||||||
|
db.flush()
|
||||||
|
db.add(Subscription(user_id=user.id, channel_id="UCgone"))
|
||||||
|
db.commit()
|
||||||
|
asked: list[list[str]] = []
|
||||||
|
monkeypatch.setattr(
|
||||||
|
subscriptions, "YouTubeClient", _youtube(asked, ["UCgone"], gone=("UCgone",))
|
||||||
|
)
|
||||||
|
|
||||||
|
import_subscriptions(db, user)
|
||||||
|
import_subscriptions(db, user)
|
||||||
|
|
||||||
|
assert asked == [["UCgone"]]
|
||||||
|
|||||||
Reference in New Issue
Block a user