Correctly add default values to created pydantic models

[kucera-lukas]

Right now default values are not being added (it's either ellipsis or None if we pass in all_optional as True). This is an issue because if people depend on the created pydantic schemas in their api endpoints the default values will not be parsed and will not be shown in openapi spec (eg. pydantic and fastapi schema generation).

[dantownsend]

This is a good idea.

The only potential issue is sometimes the default is a callable.

I can see that Pydantic's Field can either accept default or default_factory.

https://github.com/samuelcolvin/pydantic/blob/5ccbdcb5904f35834300b01432a665c75dc02296/pydantic/fields.py#L193-L196

Perhaps this would work?

params: t.Dict[str, t.Any] = {
    "default": None if is_optional else ...,
    "default_factory": column.get_default_value,
    "nullable": column._meta.null,
}

I need to dig into Pydantic some more to work out what the right approach would be.

[kucera-lukas]

Hey, that definitely makes sense. Right now I'm on a vacation but definitely gonna look into this when I get back home.

[dantownsend]

@kucera-lukas Cool, no rush - enjoy your vacation!

[kucera-lukas]

@dantownsend Hi, I'm not sure if we can make this work because as I've discovered, every piccolo Column has a default value, even if user does not specify one. For example the Integer column has a default value 0. So when creating the pydantic model we have no way of finding out whether the default has been created by the user or the library. I can't really think of any easy workaround.

Also, pydantic does not allow to pass both default and default_factory into the Field.

[dantownsend]

@kucera-lukas Thanks for looking into this.

You're right, each column always has a default.

There's another attribute called required:

class MyTable(Table):
    name = Varchar(required=True)

Piccolo itself does nothing with this attribute - it's just used to indicate to other tooling that the user should provide this value (Piccolo admin uses it for example).

Is this a potential solution? So we only set the default attribute on the Pydantic model if required = False?

[codecov-commenter]

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

[dantownsend]

Thanks a lot for this - will review and merge asap.

[dantownsend]

It's taking me a while to get this merged in - I just need to make sure this still works with the Piccolo admin.

In the old code we're setting the default to None if is_optional.