-
Notifications
You must be signed in to change notification settings - Fork 5
Description
I have just completed a full payments workflow based on @elijah0kello's pull request, #10 on the documentation. I have a number of queries related to the Pydantic models.
1. Mandatory fields not present in response data
Several models (cf OutgoingPayment, IncomingPayment, etc.) have a mandatory updatedAt field. This is not present in responses from interledger-test.dev and causes endless errors.
I don't know if you feel it's needed anywhere? If it is needed, could I replace them all with, e.g.
updatedAt: Optional[datetime] = Field(
None, description="The date and time when the incoming payment was updated."
)2. Additional fields present in response data and forbidden in the models
Multiple models have model_config that is set to forbid extra fields (cf IlpPaymentMethod, OutgoingPayment)
model_config = ConfigDict(
extra="forbid",
)Except there are additional fields sent by the identity provider, which also causes errors.
Is there any reason for this setting? If not, can it be removed?
model_config = ConfigDict()3. Deeply-nested Pydantic models
I am a little confused by the structure of the Pydantic models. There are a very large number of one-time-use fields that instead are structured as RootModel string fields.
Instead of e.g.
field_value: str = Field(..., description="Some field value")We have:
class FieldValue(RootModel[str]):
root: str = Field(..., description="A string field value")
class DeepFieldValue(RootModel[str]):
root: FieldValue = Field(..., description="A FieldValue")
class UsefulModel(BaseModel):
model_config = ConfigDict(
extra="forbid",
)
useful_field: DeepFieldValue = Field(..., description="A DeepFieldValue")This is really hard to read, and I can't imagine trying to maintain it. Should I have a go at refactoring it?
In any case, I have a working version, so we're definitely getting there. Thanks.