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

API routes should use response_model_exclude_none instead of sneaking null values into JSON #1169

Closed
ssangervasi opened this issue Oct 1, 2022 · 0 comments
Labels
bug Something isn't working

Comments

@ssangervasi
Copy link
Contributor

ssangervasi commented Oct 1, 2022

Background

API routes use Pydantic models with Optional[] fields. In Python, this is represented by None values when the field is not specified (and doesn't have a default). For these models, "optional" and "nullable" are basically the same thing.

However, for the purposes of OpenAPI schemas and JSON response bodies, "optional" means that the key may not be present in the output at all. A key that may be present with value of null must indicate that null one of of its possible types.

Issue

Route model fields like DataCategory.parent_key show up as optional (non-required) strings, meaning that they will not be included if they are unset (null/None):

Screen Shot 2022-09-30 at 16 59 19

(`*` by a field indicates it is required)

However, this route actually returns JSON objects which include parent_key: null:

  {
    description: "Data related to a system account.",
    fides_key: "account",
    name: "Account Data",
    organization_fides_key: "default_organization",
    parent_key: null,
    is_default: true,
  }

Original comment:

Still digging into the OpenApi schema, but the answer may be that we should use response_model_exclude_none on our routes to exclude optionals instead of passing nulls. Applying that to all crud endpoints would be very nice for consistency.

As-is, I think the TS generator is technically correct because if you look directly at the OpenAPI schema it says the value is optional (not required) and does not say the type can be null:
Screen Shot 2022-09-30 at 16 59 19
(* by a field indicates it is required)

I don't know of a way to get fastapi/pydantic to say the type can be null -- this didn't do it when I tried -- and I would rather exclude them anyway.

Originally posted by @ssangervasi in #1110 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants