-
Notifications
You must be signed in to change notification settings - Fork 2
Closed
Description
Summary
When constructing a FlatRatePricingOption with custom parameters, the parameters field accepts a Parameters type that contains fields from every pricing model combined:
from adcp.types import FlatRatePricingOption
po = FlatRatePricingOption(
pricing_option_id="newsletter_weekly",
pricing_model="flat_rate",
fixed_price=250.00,
currency="USD",
parameters={"unit": "week", "sends_per_week": 5},
)
# The validated parameters object contains unrelated fields:
print(po.parameters)
# Parameters(
# daypart=None,
# duration_hours=None,
# estimated_impressions=None,
# loop_duration_seconds=None, # DOOH field
# min_plays_per_hour=None, # DOOH field
# sov_percentage=None, # DOOH field
# venue_package=None, # DOOH field
# unit='week',
# sends_per_week=5,
# )Problem
- Autocomplete is misleading — IDE suggests
sov_percentageandvenue_packageon a flat-rate email newsletter pricing option - No validation — You can set
loop_duration_secondson a flat-rate option and it silently accepts it - Schema is confusing — A consumer reading the schema can't tell which parameters apply to which pricing model
Suggested Fix
Discriminated parameter types per pricing model:
class FlatRateParameters(BaseModel):
unit: str # "week", "month", "event", "quarter"
# Other flat-rate-specific fields
class DoohParameters(BaseModel):
loop_duration_seconds: int | None = None
min_plays_per_hour: int | None = None
sov_percentage: float | None = None
venue_package: str | None = None
class TimeBasedParameters(BaseModel):
duration_hours: int | None = None
daypart: str | None = NoneThis is likely a spec-level change (affects pricing-option.json), but the Python library could add validation even if the shared schema stays flat — e.g., a model validator that warns/strips inapplicable fields based on pricing_model.
Impact
Low-to-medium. This is more of a code quality / developer confusion issue than a blocker. But it makes the library feel auto-generated rather than designed, which hurts trust when building against it.
Environment
- adcp: 3.6.0
- Python: 3.12
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels