feat(handlers): expose sync_catalogs through sales-catalog-driven specialism#838
Draft
bokelley wants to merge 2 commits into
Draft
feat(handlers): expose sync_catalogs through sales-catalog-driven specialism#838bokelley wants to merge 2 commits into
bokelley wants to merge 2 commits into
Conversation
…tion note, example - Add sync_catalogs stub to _SyncSalesPlatform (SalesPlatform @runtime_checkable now requires the method for isinstance to return True) - Add 6.1.0-beta.1 migration note documenting the isinstance behavior change - Add examples/hello_seller_catalog.py showing discovery mode, delete_missing guard, and ergonomic list return
| (ergonomic form) or a fully-shaped | ||
| :class:`~adcp.types.SyncCatalogsSuccessResponse`. | ||
| """ | ||
| ... |
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.
Closes #786
Re-cut of #790 from current main (
19fde5e). PR #790 was conflicting (dirty) with maintainer edits disabled; this branch is a clean re-apply of the same logic on top of6.1.0-beta.1.Summary
sync_catalogsintoSPECIALISM_TO_ADVERTISED_TOOLSsosales-catalog-drivenplatforms expose the tool viatools/listPlatformHandler.sync_catalogsdispatcher — fullSyncCatalogsRequestpassed through (noarg_projector) so adopters can inspectreq.delete_missing,req.dry_run,req.validation_mode, andreq.catalog_idssync_catalogstoSalesPlatformProtocol body (required section,sales-catalog-drivenonly)_require_platform_method("sync_catalogs")guard — non-catalog-driven platforms that somehow receive the call surfaceUNSUPPORTED_FEATUREinstead ofINTERNAL_ERRORsync_catalogsto_OPTIONAL_PLATFORM_METHODS(specialism-gated variant — absent on all non-sales-catalog-drivenplatforms)examples/hello_seller_catalog.py— runnable minimal adopter with discovery mode,delete_missingguard, and ergonomic list returnMIGRATION_v5_to_v6.md6.1.0-beta.1 entry documentingSalesPlatform@runtime_checkableisinstancebehavior change_SyncSalesPlatformtest fixture to remain structurally conformant withSalesPlatformProtocol note:
sync_catalogson the sharedSalesPlatformProtocolAdding
sync_catalogsto the sharedSalesPlatformProtocol (rather than a separateCatalogDrivenPlatformmixin) is intentional and consistent with howsync_audiencesis handled onAudiencePlatform. The consequence is thatisinstance(platform, SalesPlatform)now requiressync_catalogsfor structural conformance — non-catalog-driven platforms that use this check need a stub. This is documented inMIGRATION_v5_to_v6.md. A separateCatalogDrivenPlatformmixin would be cleaner but is a larger structural change; see code-reviewer sign-off below.What was tested
pytest tests/test_decisioning_advertised_per_specialism.py tests/test_decisioning_dispatch.py tests/test_decisioning_handler_shims.py tests/test_decisioning_specialisms.py tests/test_lazy_platform_router.py— 235 passedruff check src/— cleanpython -m mypy src/adcp/decisioning/handler.py src/adcp/decisioning/specialisms/sales.py— cleanNits (not fixing in this PR):
_require_platform_methodmessage says "optional method" — technically imprecise forsync_catalogsonsales-catalog-driven, butvalidate_platform(called fromserve()) hard-fails at boot before any buyer reaches this gate; the backstop is only reachable via programmaticPlatformHandlerconstruction withoutserve(). Pre-existing pattern used by 8 other methods._maybe_auto_emit_sync_completionfires ondry_run=Truerequests — pre-existing pattern shared withsync_audiences;sync_catalogsinwebhook_emit._SYNC_COMPLETION_METHODSis already present on main.MaybeAsync[SyncCatalogsSuccessResponse]Protocol return type doesn't formally include the ergonomiclist[SyncCatalogResult]arm — pre-existing mismatch shared withsync_audiences.Pre-PR review
code-reviewer: No blockers. Design concern noted (shared Protocol body vs. separate mixin) — explained above as intentional pattern. 1 nit (inline fixture duplication across 3 shim tests). Approved.
dx-expert: Approved (2nd pass). Previous blockers resolved: example added,
_SyncSalesPlatformfixture fixed, migration note added. No new blockers.Session: https://claude.ai/code/session_01NYBCVdwct1Hhz7GD1eaEHr
Generated by Claude Code