Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

annotate response model? #202

Closed
wmshort opened this issue Aug 28, 2021 · 6 comments · Fixed by #204
Closed

annotate response model? #202

wmshort opened this issue Aug 28, 2021 · 6 comments · Fixed by #204

Comments

@wmshort
Copy link
Contributor

wmshort commented Aug 28, 2021

Integrating Piccolo ORM with FastAPI! It's a joy. However, I'm stumbling over a situation where I want to annotate a model response object with an additional key/value pair. (The response models have been automatically generated through create_pydantic_model, so I don't see a way of adding fields except by subclassing -- but the kind of annotations I want are more transient than this). The API is returning models from the database via .select(), and it's here that I would want to add these transient annotations.

Imagine, as it were:

class PersonModelOut:          # but created by create_pydantic_model, based on Piccolo ORM table
    name: str
    age: int

....in the API, where the response model is PersonModelOut:
return Person.select(Person.name, Person.age, Person.annotation( { "disposition": "cranky" } ).where(Person.age > 40).run()

Or is this totally off the wall?

@dantownsend
Copy link
Member

There's a couple of solutions I can think of, but I haven't tried them out.

One option is a JSON field.

from pydantic import BaseModel, Json

class MyModel(BaseModel):
    id: int
    extra: Json  # Or could also be str

That way you can add extra return data into the JSON field.

The other approach is using optional fields in Pydantic:

import typing as t

from pydantic import BaseModel

class MyModel(BaseModel):
    id: int
    extra: t.Optional[str]

Not sure if either are what you're after.

@wmshort
Copy link
Contributor Author

wmshort commented Aug 28, 2021

The optional field is a pretty good solution for this immediate issue, thanks!

Incidentally, if I have a (purely hypothetical and invented) set of Pydantic models like this:

class Hobby(BaseModel):
     name: str
     hours_wasted: int

     class Config:
        orm_mode = True

class Person(BaseModel);
    name: str
    hobby: Hobby

    class Config:
        orm_mode = True

where one model is embedded in the other, if I have a set of ORM objects, how does one 'follow through' the foreign keys on the Table models (corresponding to, in this example, Person.hobby), so that the embedded Pydantic models become populated with the relevant values as well as the top-level model (rather than with merely the id of the referenced object in the database?) get_related() works on single ORM objects, not on whole queries, unless I'm missing something (very possibly)?

@dantownsend
Copy link
Member

Piccolo doesn't automatically traverse foreign keys like in Django, as it can cause performance issues (N+1 queries).

Performance wise, it's best to get all of the data in one go. You can do something like this:

import pydantic


class BandModel(pydantic.BaseModel):
    name: str
    manager_name: str = pydantic.Field(alias="manager.name")


data = Band.select(Band.name, Band.manager.name).first().run_sync()
model = MyModel(**data)
>>> model.name
'Pythonistas'
>>> model.manager_name
'Guido'

If you want all of the data from the related table, you can use:

>>> data = Band.select(Band.name, *Band.manager.all_columns()).first().run_sync()
{'name': 'Pythonistas', 'manager.id': 1, 'manager.name': 'Guido'}

All of the data in the Pydantic model would be flat though, and doesn't have a nested model.

For Piccolo to support this type of nesting, it would require an API change. Something like:

>>> Band.select(Band.name, *Band.manager.all_columns()).output(nested=True).first().run_sync()
{'name': 'Pythonistas', 'band': {'id': 1, 'name': 'Guido'}}

That way you should be able to pass it straight to a Pydantic model containing nested models. What do you think?

@wmshort
Copy link
Contributor Author

wmshort commented Aug 29, 2021

I think that's great. Presumably you'd end up with something like

{
    "name": "Bob",
    "hobby": {
                        "name": "whittling",
                        "hours_wasted": 1234,
                    }
    }

which would then fill out the nested Pydantic models normally?

For the added option, I had originally imagined fetch_related by way of Django inspiration; however that refers to model objects, so output or something similar makes sense; and could provide a space for the addition of other options later (output(nested=True, other_option=False, yet_another_option_I_haven_t_even_conceived_yet=3))

@dantownsend
Copy link
Member

That's right. I think this would be a valuable feature. I'll try and add it.

It would be worth upgrading create_pydantic_model in a similar way, so it can also generate nested models.

create_pydantic_model(Band, nested_tables=[Band.manager])

@dantownsend
Copy link
Member

The changes to Piccolo API are in progress still, but almost finished. Will get those merged tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants