fix(compat): warn on non-standard brand_manifest path in inline-object branch#688
Merged
bokelley merged 1 commit intoMay 12, 2026
Merged
Conversation
…t branch The non-standard-path warning was applied only on the v2.5 brand_manifest URL-string branch. The inline BrandManifest object's url field can carry the same path (v2.5 spec example points at https://cdn.example.com/brand-manifest.json), and the same information loss applies — v3 sellers will fetch the canonical {domain}/.well-known/brand.json path and likely 404. Centralize the warning helper + dedup state in _url.py alongside extract_brand_domain so get_products and _media_buy_helpers share one implementation. Both adapters now call warn_brand_manifest_path_lossy from both the string and the inline-object branches. Fixes #687 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
Expert reviewcode-reviewer: approved — no blockers. Confirmed:
Non-blocking nits filed for future work:
CI: 15/15 green. Merging. |
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.
Summary
Fixes #687. After #685 (inline-object branch) and #686 (non-standard-path warning) landed, the warning was applied only to the URL-string branch. The v2.5
brand-manifest-refschema allowsbrand_manifestto be either a URL string or an inlineBrandManifestobject — the inline object'surlfield carries the same path semantics, and the same information loss applies (v3 sellers fetch the canonical{domain}/.well-known/brand.jsonand will likely 404).Changes
src/adcp/compat/legacy/v2_5/_url.py: newwarn_brand_manifest_path_lossy(manifest_url, domain)function plus the_STANDARD_BRAND_MANIFEST_PATHSconstant and_brand_manifest_path_warneddedup set (moved from per-adapter modules). Logger isadcp.compat.legacy.v2_5._url.src/adcp/compat/legacy/v2_5/get_products.pyand_media_buy_helpers.py: remove the per-module helpers; callwarn_brand_manifest_path_lossyfrom both the string and inline-object branches.Test plan
pytest tests/test_legacy_adapter_v2_5_*.py— 64 passed (was 60; +4 new dict-branch warning tests)assert 0 == 1(no warning emitted)ruff checkandmypy src/adcp/compat/legacy/v2_5/cleanLogger-name change
Operators previously alerting on
adcp.compat.legacy.v2_5.get_products/adcp.compat.legacy.v2_5._media_buy_helperswarnings will need to update the logger name toadcp.compat.legacy.v2_5._url. The warning message text is unchanged, so grep-based alerting on "non-standard path" is unaffected. Acceptable shift since #686 landed only an hour ago and adoption of log-based alerting on this specific warning is unlikely.