You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Inline BrandManifest object with optional url field — translated to brand.domain via extract_brand_domain(url), but the non-standard-path warning is NOT applied.
The same information loss occurs in both cases: a non-canonical path on the inline-object's url field will be silently flattened to the hostname, and v3 sellers' canonical /.well-known/brand.json fetch will likely 404.
Suggested fix
Extract the warning logic into a shared helper (e.g. warn_if_brand_manifest_path_lossy in src/adcp/compat/legacy/v2_5/_url.py alongside extract_brand_domain) and call it from both branches in both adapters.
This also addresses the duplication nit code-reviewer raised on #686 (warning logic copy-pasted across get_products.py and _media_buy_helpers.py).
Bounded dedup cache (_brand_manifest_path_warned is currently an unbounded set) — separate follow-up worth filing if production cardinality ever becomes a concern.
Background
Surfaced during rebase of PR #686 (#683) on top of PR #685 (#684), and called out by
ad-tech-protocol-expertreview of #686.After both PRs merge, the v2.5 → v3
brand_manifestadapter handles two input shapes:BrandManifestobject with optionalurlfield — translated tobrand.domainviaextract_brand_domain(url), but the non-standard-path warning is NOT applied.The same information loss occurs in both cases: a non-canonical path on the inline-object's
urlfield will be silently flattened to the hostname, and v3 sellers' canonical/.well-known/brand.jsonfetch will likely 404.Suggested fix
Extract the warning logic into a shared helper (e.g.
warn_if_brand_manifest_path_lossyinsrc/adcp/compat/legacy/v2_5/_url.pyalongsideextract_brand_domain) and call it from both branches in both adapters.This also addresses the duplication nit code-reviewer raised on #686 (warning logic copy-pasted across
get_products.pyand_media_buy_helpers.py).Affected call sites
src/adcp/compat/legacy/v2_5/get_products.py:adapt_request— inline-object branchsrc/adcp/compat/legacy/v2_5/_media_buy_helpers.py:adapt_brand_manifest_to_brand— inline-object branchOut of scope
Bounded dedup cache (
_brand_manifest_path_warnedis currently an unboundedset) — separate follow-up worth filing if production cardinality ever becomes a concern.