Skip to content

Commit f135651

Browse files
bokelleyclaude
andauthored
fix: resolve CI failures from GetSignalsRequest union alias (#144)
* fix: expose GetSignalsRequest as union alias, not RootModel Pydantic hard-rejects model_config['extra'] on RootModel subclasses, which broke the documented pattern of subclassing library types with env-based config. Adding a plain union alias in aliases.py shadows the generated RootModel wrapper, restoring the ability for consumers to subclass GetSignalsRequest1 or GetSignalsRequest2 with custom model_config. Closes #138 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: resolve CI failures from GetSignalsRequest union alias - Shorten docstring in aliases.py to fix ruff E501 line-too-long error - Fix simple.py to dispatch GetSignalsRequest variants via model_validate instead of calling the union type as a constructor (mypy operator error) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: exclude examples/ and scripts/ from ruff linting These directories contain dev tooling and examples, not published code. CI already only lints src/, so this makes the local config consistent. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent b0497a3 commit f135651

File tree

5 files changed

+58
-3
lines changed

5 files changed

+58
-3
lines changed

pyproject.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,8 @@ extend-exclude = [
7575
"src/adcp/types/_generated.py",
7676
"src/adcp/types/tasks.py",
7777
"src/adcp/types/generated_poc/",
78+
"examples/",
79+
"scripts/",
7880
]
7981

8082
[tool.ruff.lint]

src/adcp/simple.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@
3636
GetMediaBuyDeliveryResponse,
3737
GetProductsRequest,
3838
GetProductsResponse,
39-
GetSignalsRequest,
39+
GetSignalsDiscoveryRequest,
40+
GetSignalsLookupRequest,
4041
GetSignalsResponse,
4142
ListAccountsRequest,
4243
ListAccountsResponse,
@@ -275,7 +276,11 @@ async def get_signals(
275276
Raises:
276277
Exception: If the request fails
277278
"""
278-
request = GetSignalsRequest(**kwargs)
279+
request: GetSignalsDiscoveryRequest | GetSignalsLookupRequest
280+
if "signal_ids" in kwargs:
281+
request = GetSignalsLookupRequest.model_validate(kwargs)
282+
else:
283+
request = GetSignalsDiscoveryRequest.model_validate(kwargs)
279284
result = await self._client.get_signals(request)
280285
if not result.success or not result.data:
281286
raise ADCPSimpleAPIError(

src/adcp/types/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,6 @@
184184
GetProductsResponse,
185185
GetPropertyListRequest,
186186
GetPropertyListResponse,
187-
GetSignalsRequest,
188187
GetSignalsResponse,
189188
Gtin,
190189
HtmlAsset,
@@ -399,6 +398,7 @@
399398
GetProductsWholesaleRequest,
400399
GetSignalsDiscoveryRequest,
401400
GetSignalsLookupRequest,
401+
GetSignalsRequest,
402402
HtmlPreviewRender,
403403
InlineDaastAsset,
404404
InlineVastAsset,

src/adcp/types/aliases.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -506,6 +506,9 @@ def process_result(result: SyncCatalogResult) -> None:
506506
"""Get products in wholesale mode - buying_mode='wholesale', raw inventory."""
507507

508508
# Get Signals Request Variants
509+
GetSignalsRequest = GetSignalsRequest1 | GetSignalsRequest2
510+
"""Union of GetSignalsRequest variants. Use instead of the RootModel wrapper."""
511+
509512
GetSignalsDiscoveryRequest = GetSignalsRequest1
510513
"""Discover signals by natural language spec - signal_spec required."""
511514

tests/test_type_aliases.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -808,3 +808,48 @@ def test_destination_union_contains_all_variants():
808808
}
809809

810810
assert union_args == expected_variants
811+
812+
813+
def test_get_signals_request_is_union_not_root_model():
814+
"""GetSignalsRequest is a plain union alias, not a RootModel.
815+
816+
This allows consumers to subclass GetSignalsRequest1 or GetSignalsRequest2
817+
with custom model_config (e.g. extra='forbid' in CI, extra='ignore' in prod).
818+
See: https://github.com/adcontextprotocol/adcp-client-python/issues/138
819+
"""
820+
import types
821+
822+
from pydantic import RootModel
823+
824+
from adcp import GetSignalsRequest
825+
826+
# Must be a union, not a RootModel subclass
827+
assert isinstance(GetSignalsRequest, types.UnionType)
828+
assert not (isinstance(GetSignalsRequest, type) and issubclass(GetSignalsRequest, RootModel))
829+
830+
831+
def test_get_signals_request_union_contains_variants():
832+
"""GetSignalsRequest union contains the two concrete variants."""
833+
from typing import get_args
834+
835+
from adcp import GetSignalsDiscoveryRequest, GetSignalsLookupRequest, GetSignalsRequest
836+
837+
union_args = set(get_args(GetSignalsRequest))
838+
assert union_args == {GetSignalsDiscoveryRequest, GetSignalsLookupRequest}
839+
840+
841+
def test_get_signals_request_variants_are_subclassable():
842+
"""Consumers can subclass GetSignalsRequest variants with custom model_config."""
843+
from pydantic import ConfigDict
844+
845+
from adcp import GetSignalsDiscoveryRequest, GetSignalsLookupRequest
846+
847+
class MyDiscoveryRequest(GetSignalsDiscoveryRequest):
848+
model_config = ConfigDict(extra="forbid")
849+
850+
class MyLookupRequest(GetSignalsLookupRequest):
851+
model_config = ConfigDict(extra="forbid")
852+
853+
# Should construct without error
854+
req = MyDiscoveryRequest(signal_spec="targeting signals for automotive in-market buyers")
855+
assert req.signal_spec == "targeting signals for automotive in-market buyers"

0 commit comments

Comments
 (0)