feat: sync models from UCP and update sdk version to 0.3#24
feat: sync models from UCP and update sdk version to 0.3#24cusell-google wants to merge 6 commits intoUniversal-Commerce-Protocol:mainfrom
Conversation
Update the version of the SDK to 0.3. This version is larger than the previous one which was 0.1.0.
cusell-google
requested review from
jingyli,
nicholasjameshall,
ptiper and
yanheChen
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly expands the UCP Python SDK by introducing comprehensive shopping models, including cart management, catalog search/lookup, and identity linking. It refactors existing schemas to support hierarchical extensions, adds detailed types for pricing, fulfillment, and product metadata, and moves extension-specific checkout models to a dedicated namespace. Feedback focuses on ensuring model consistency with documentation, specifically regarding required fields for currency and amounts, enforcing structural constraints via validators, and consolidating redundant generated types.
| model_config = ConfigDict( | ||
| extra="allow", | ||
| ) | ||
| amount: signed_amount.SignedAmount | None = None |
There was a problem hiding this comment.
The amount field is marked as optional with a default of None, but the docstring states that platforms MUST render all entries using the amount. Additionally, the sum of line amounts must equal this parent amount. Making it optional contradicts these requirements and could lead to inconsistencies. Since this is a core part of the pricing breakdown, it should be a required field.
| amount: signed_amount.SignedAmount | None = None | |
| amount: signed_amount.SignedAmount |
| """ | ||
|
|
||
|
|
||
| class Totals(RootModel[list[Total]]): |
There was a problem hiding this comment.
The Totals model docstring specifies that it MUST contain exactly one subtotal and one total entry. This constraint is currently not enforced by the code. Consider adding a Pydantic validator to ensure compliance with the UCP specification and prevent invalid pricing breakdowns from being processed.
| """ | ||
| Content format, default = plain. | ||
| """ | ||
| presentation: str | None = "notice" |
There was a problem hiding this comment.
The presentation field has a specific set of well-known values ('notice', 'disclosure') described in the docstring. Using a Literal type would provide better validation and IDE support for SDK users.
| presentation: str | None = "notice" | |
| presentation: Literal["notice", "disclosure"] | None = "notice" |
| class Extends(RootModel[str]): | ||
| model_config = ConfigDict( | ||
| frozen=True, | ||
| ) | ||
| root: str = Field(..., pattern="^[a-z][a-z0-9]*(?:\\.[a-z][a-z0-9_]*)+$") | ||
| """ | ||
| Parent capability(s) this extends. Present for extensions, absent for root capabilities. Use array for multi-parent extensions. | ||
| """ | ||
|
|
||
|
|
||
| class Extends1Item(RootModel[str]): | ||
| model_config = ConfigDict( | ||
| frozen=True, | ||
| ) | ||
| root: str = Field(..., pattern="^[a-z][a-z0-9]*(?:\\.[a-z][a-z0-9_]*)+$") | ||
|
|
||
|
|
||
| class Extends1(RootModel[list[Extends1Item]]): | ||
| model_config = ConfigDict( | ||
| frozen=True, | ||
| ) | ||
| root: list[Extends1Item] = Field(..., min_length=1) | ||
| """ | ||
| Parent capability(s) this extends. Present for extensions, absent for root capabilities. Use array for multi-parent extensions. | ||
| """ | ||
|
|
||
|
|
||
| class Extends2(RootModel[str]): | ||
| model_config = ConfigDict( | ||
| frozen=True, | ||
| ) | ||
| root: str = Field(..., pattern="^[a-z][a-z0-9]*(?:\\.[a-z][a-z0-9_]*)+$") | ||
| """ | ||
| Parent capability(s) this extends. Present for extensions, absent for root capabilities. Use array for multi-parent extensions. | ||
| """ | ||
|
|
||
|
|
||
| class Extends3Item(RootModel[str]): | ||
| model_config = ConfigDict( | ||
| frozen=True, | ||
| ) | ||
| root: str = Field(..., pattern="^[a-z][a-z0-9]*(?:\\.[a-z][a-z0-9_]*)+$") | ||
|
|
||
|
|
||
| class Extends3(RootModel[list[Extends3Item]]): | ||
| model_config = ConfigDict( | ||
| frozen=True, | ||
| ) | ||
| root: list[Extends3Item] = Field(..., min_length=1) | ||
| """ | ||
| Parent capability(s) this extends. Present for extensions, absent for root capabilities. Use array for multi-parent extensions. | ||
| """ | ||
|
|
||
|
|
||
| class Extends4(RootModel[str]): | ||
| model_config = ConfigDict( | ||
| frozen=True, | ||
| ) | ||
| root: str = Field(..., pattern="^[a-z][a-z0-9]*(?:\\.[a-z][a-z0-9_]*)+$") | ||
| """ | ||
| Parent capability(s) this extends. Present for extensions, absent for root capabilities. Use array for multi-parent extensions. | ||
| """ | ||
|
|
||
|
|
||
| class Extends5Item(RootModel[str]): | ||
| model_config = ConfigDict( | ||
| frozen=True, | ||
| ) | ||
| root: str = Field(..., pattern="^[a-z][a-z0-9]*(?:\\.[a-z][a-z0-9_]*)+$") | ||
|
|
||
|
|
||
| class Extends5(RootModel[list[Extends5Item]]): | ||
| model_config = ConfigDict( | ||
| frozen=True, | ||
| ) | ||
| root: list[Extends5Item] = Field(..., min_length=1) | ||
| """ | ||
| Parent capability(s) this extends. Present for extensions, absent for root capabilities. Use array for multi-parent extensions. | ||
| """ | ||
|
|
||
|
|
||
| class Extends6(RootModel[str]): | ||
| model_config = ConfigDict( | ||
| frozen=True, | ||
| ) | ||
| root: str = Field(..., pattern="^[a-z][a-z0-9]*(?:\\.[a-z][a-z0-9_]*)+$") | ||
| """ | ||
| Parent capability(s) this extends. Present for extensions, absent for root capabilities. Use array for multi-parent extensions. | ||
| """ | ||
|
|
||
|
|
||
| class Extends7Item(RootModel[str]): | ||
| model_config = ConfigDict( | ||
| frozen=True, | ||
| ) | ||
| root: str = Field(..., pattern="^[a-z][a-z0-9]*(?:\\.[a-z][a-z0-9_]*)+$") | ||
|
|
||
|
|
||
| class Extends7(RootModel[list[Extends7Item]]): | ||
| model_config = ConfigDict( | ||
| frozen=True, | ||
| ) | ||
| root: list[Extends7Item] = Field(..., min_length=1) | ||
| """ | ||
| Parent capability(s) this extends. Present for extensions, absent for root capabilities. Use array for multi-parent extensions. | ||
| """ |
There was a problem hiding this comment.
There is significant duplication of Extends types (e.g., Extends, Extends2, Extends4, Extends6 are identical RootModel[str] definitions). This appears to be an artifact of the code generation process. Consolidating these into a single reusable type would improve the maintainability and readability of the SDK models.
| Append-only event log of money movements (refunds, returns, credits, disputes, cancellations, etc.) that exist independently of fulfillment. | ||
| """ | ||
| totals: list[total.Total] | ||
| currency: str | None = None |
There was a problem hiding this comment.
The currency field is marked as optional, but the docstring states it MUST match the currency from the originating checkout session. If this is a mandatory requirement for the order representation, it should be a required field in the model to ensure data completeness.
1 participant
sync python-sdk from ucp and update version to 0.3. This version update is larger than the previous one which was 0.1.0.