-
Notifications
You must be signed in to change notification settings - Fork 96
Description
When creating pydantic models with create_pydantic_model function, columns marked as null are not marked as null in the data. Instead the function uses the required property.
How to recreate
Consider the following application:
from piccolo.table import Table
from piccolo.utils.pydantic import create_pydantic_model
from piccolo import columns
# We need something that consumes the Pydantic models
from fastapi import FastAPI
import json
class SomeThing(Table):
name = columns.Text(null=False)
pet_name = columns.Text(null=True)
age = columns.Integer(required=True, null=True)
some_model = create_pydantic_model(SomeThing)
app = FastAPI()
@app.get('/')
def foo() -> some_model:
pass
# We print the OpenAPI schema generated for table `SomeThing`
print(json.dumps(app.openapi()['components']['schemas']['SomeThing']))The output returned is:
{
"properties": {
"name": {
"anyOf": [
{
"type": "string"
},
{
"type": "null"
}
],
"title": "Name",
"extra": {
"nullable": false,
"secret": false,
"unique": false,
"widget": "text-area"
}
},
"pet_name": {
"anyOf": [
{
"type": "string"
},
{
"type": "null"
}
],
"title": "Pet Name",
"extra": {
"nullable": true,
"secret": false,
"unique": false,
"widget": "text-area"
}
},
"age": {
"type": "integer",
"title": "Age",
"extra": {
"nullable": true,
"secret": false,
"unique": false
}
}
},
"type": "object",
"required": [
"age"
],
"title": "SomeThing",
"extra": {}
}As we can see, the fields name and pet_name are marked as str | None, despite having different null options set.
In contrast, the age field is marked as an int, which may never be None, despite it being possibly null in the database.
What causes this behavior:
piccolo/piccolo/utils/pydantic.py
Line 246 in 7373a84
| is_optional = True if all_optional else not column._meta.required |
In the above line, the type for the field is set as T | None based on the required property, instead of the null property.
This generates plausible models for writing to the database, but not for reading data from the database (we get rogue nullable properties).
Why does this behavior need to be changed:
Let's consider a simple change to the example above:
from uuid import uuid4
from piccolo.table import Table
from piccolo.utils.pydantic import create_pydantic_model
from piccolo import columns
from fastapi import FastAPI
import json
class SomeThing(Table):
id = columns.UUID(default=uuid4(), null=False)
some_model = create_pydantic_model(SomeThing)
app = FastAPI()
@app.get('/')
def foo() -> some_model:
pass
print(json.dumps(app.openapi()['components']['schemas']['SomeThing']))We have an autogenerated uuid column, and we would like to create a model that describes a row in that table. We cannot mark id as required as it might be generated by the database.
The generated OpenAPI schema is:
{
"properties": {
"id": {
"anyOf": [
{
"type": "string",
"format": "uuid"
},
{
"type": "null"
}
],
"title": "Id",
"extra": {
"nullable": false,
"secret": false,
"unique": false
}
}
},
"type": "object",
"title": "SomeThing",
"extra": {}
}
This causes various OpenAPI client generators to generate the following type:
interface SomeThing {
id: string | null
}This makes it seem that the id might be null, when it will never be null.
How to fix this bug:
For my purposes, I just run this patch:
This causes piccolo to generate the correct models for read operations.
For write purposes, I just use all_optional.