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

Fix models to support pydantic orm_mode #43

Closed
wants to merge 2 commits into from

Conversation

ac3d912
Copy link

@ac3d912 ac3d912 commented Jul 15, 2019

Null (None) values will cause ValidationErrors when used with postgres.
See: tiangolo/fastapi#361 (comment)

I made these changes and it works like a charm. (The issue out-of-the-box being the None value for first superuser's Full Name)

FastAPI version: 0.33.0
Pydantic version: 0.30

Example validation error

INFO: ('192.168.0.2', 53732) - "POST /api/v1/login/test-token HTTP/1.1" 500
ERROR: Exception in ASGI application
Traceback (most recent call last):
  File "/usr/local/lib/python3.7/site-packages/uvicorn/protocols/http/httptools_impl.py", line 368, in run_asgi
    result = await app(self.scope, self.receive, self.send)
  File "/usr/local/lib/python3.7/site-packages/starlette/applications.py", line 133, in __call__
    await self.error_middleware(scope, receive, send)
  File "/usr/local/lib/python3.7/site-packages/starlette/middleware/errors.py", line 122, in __call__
    raise exc from None
  File "/usr/local/lib/python3.7/site-packages/starlette/middleware/errors.py", line 100, in __call__
    await self.app(scope, receive, _send)
  File "/usr/local/lib/python3.7/site-packages/starlette/middleware/base.py", line 25, in __call__
    response = await self.dispatch_func(request, self.call_next)
  File "./app/main.py", line 34, in db_session_middleware
    response = await call_next(request)
  File "/usr/local/lib/python3.7/site-packages/starlette/middleware/base.py", line 45, in call_next
    task.result()
  File "/usr/local/lib/python3.7/site-packages/starlette/middleware/base.py", line 38, in coro
    await self.app(scope, receive, send)
  File "/usr/local/lib/python3.7/site-packages/starlette/middleware/cors.py", line 84, in __call__
    await self.simple_response(scope, receive, send, request_headers=headers)
  File "/usr/local/lib/python3.7/site-packages/starlette/middleware/cors.py", line 140, in simple_response
    await self.app(scope, receive, send)
  File "/usr/local/lib/python3.7/site-packages/starlette/exceptions.py", line 73, in __call__
    raise exc from None
  File "/usr/local/lib/python3.7/site-packages/starlette/exceptions.py", line 62, in __call__
    await self.app(scope, receive, sender)
  File "/usr/local/lib/python3.7/site-packages/starlette/routing.py", line 585, in __call__
    await route(scope, receive, send)
  File "/usr/local/lib/python3.7/site-packages/starlette/routing.py", line 207, in __call__
    await self.app(scope, receive, send)
  File "/usr/local/lib/python3.7/site-packages/starlette/routing.py", line 40, in app
    response = await func(request)
  File "/usr/local/lib/python3.7/site-packages/fastapi/routing.py", line 122, in app
    skip_defaults=response_model_skip_defaults,
  File "/usr/local/lib/python3.7/site-packages/fastapi/routing.py", line 54, in serialize_response
    raise ValidationError(errors)
pydantic.error_wrappers.ValidationError: 1 validation error
response
  value is not a valid dict (type=type_error.dict)

@jf---
Copy link

jf--- commented Aug 16, 2019

I made these changes and it works like a charm

Happy to second that ;)

I think its fairly urgent to merge this PR ( @tiangolo ), since it does break the test suite


perhaps running CI nightly is of interest, that'll help catching upstream changes
given the in the Pipfile the exact version of the dependencies isn't specified, this might help to catch such breaking changes.

@bossjones
Copy link

I came across the same issue, thank you for posting a PR @ac3d912. Also thank you @tiangolo for this project.

@victorbordo
Copy link

Just ran into this issue as well. Appreciate you identifying it and submitting a PR @ac3d912. Saved me a lot of time.

FastAPI is awesome @tiangolo. Thanks for all the work you've put into creating and maintaining it.

@stratosgear
Copy link

Here is a 👍 from me too, for this lifesaving tip! This ought to be merged!

@stratosgear
Copy link

Also regarding @jf--- comment about upstream changes, I hope my recent #69 will be taken into consideration, as it will potentially eliminate such upstream cases...?

@rlonka
Copy link
Contributor

rlonka commented Oct 24, 2019

I have ran into this issue as well.
Thanks @tiangolo for great project!

br3ndonland added a commit to br3ndonland/full-stack-fastapi-postgresql that referenced this pull request Dec 6, 2019
See GitHub PR tiangolo#43
tiangolo#43
See GitHub issue tiangolo#72
tiangolo#72

When viewing the user profile, name and email weren't showing up.

Docker Compose logs showed a pydantic error:
https://pydantic-docs.helpmanual.io/

```
backend_1        | pydantic.error_wrappers.ValidationError: 1 validation
                   error for User
backend_1        | response -> 0
backend_1        |   value is not a valid dict (type=type_error.dict)
```

Pydantic orm_mode is needed. This commit will update the database models
to use pydantic orm_mode, with cleaner syntax than the fix suggested in
issue tiangolo#72 (by adding class Config directly to the base class).
@StephenBrown2
Copy link
Contributor

@tiangolo Fixed in #23?

@tiangolo
Copy link
Owner

tiangolo commented Feb 7, 2020

Thanks @ac3d912 for debugging and implementing it! 🎉 🚀

Thanks everyone for the discussion here (and the patience 😅 ).

@StephenBrown2 , yep, thanks. This was fixed as part of that PR.

I'll close this PR now as the fix is already implemented (from a different PR).

@tiangolo tiangolo closed this Feb 7, 2020
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 this pull request may close these issues.

None yet

8 participants