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 issue for SQLAlchemy Dataclasses post 0.67 #4094

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

Lunrtick
Copy link

@Lunrtick Lunrtick commented Oct 26, 2021

edited due to change from flag based approach in the route to an optional property on the dataclasses.

As detailed in #3616, SQLAlchemy dataclasses don't work nicely with the new defaults introduced in FastAPI 0.67. This pull request adds the option for dataclasses to include a read_with_orm_mode flag, matching the options available to Pydantic models.

Currently this is specified as a special dunder property: __read_with_orm_mode__ on each dataclass, though it could take a different form. In my codebases, all my SQLAlchemy dataclasses inherit from the same base class, so I would only be setting this property on the base class.

Besides disabling the new dataclass parsing feature in some manner, it seems as though it'd be impossible to use the SQLAlchemy dataclass pattern as described in their docs here.

This allows for a workaround for SQLAlchemy dataclasses after the changes introduced in 0.67 .

It replicates the funcionality provided for Pydantic models (`read_with_orm_mode`) by allowing a special property to be set on dataclasses that want to opt in to the feature.
@Lunrtick
Copy link
Author

Another option would be to check if a response model was declared - I'd imagine that automatically using the dataclass doesn't make sense when an explicit Pydantic response model is given, though I may have misunderstood.

@codecov
Copy link

codecov bot commented May 5, 2022

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (cf73051) 100.00% compared to head (be25e18) 99.99%.
Report is 1038 commits behind head on master.

❗ Current head be25e18 differs from pull request most recent head 77590d7. Consider uploading reports for the commit 77590d7 to get more accurate results

Files Patch % Lines
fastapi/routing.py 66.66% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##            master    #4094      +/-   ##
===========================================
- Coverage   100.00%   99.99%   -0.01%     
===========================================
  Files          540      532       -8     
  Lines        13969    13687     -282     
===========================================
- Hits         13969    13686     -283     
- Misses           0        1       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@github-actions
Copy link
Contributor

github-actions bot commented May 5, 2022

📝 Docs preview for commit 073fa49 at: https://62739d3ddce123056c9bbac6--fastapi.netlify.app

@github-actions
Copy link
Contributor

github-actions bot commented May 6, 2022

📝 Docs preview for commit 1dae9ef at: https://6274d36709ac783c1238b259--fastapi.netlify.app

@github-actions
Copy link
Contributor

📝 Docs preview for commit f019ca3 at: https://628b80ed0c8e944d43fda51c--fastapi.netlify.app

@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2022

📝 Docs preview for commit be25e18 at: https://62e7a158fb289d243474133d--fastapi.netlify.app

@github-actions
Copy link
Contributor

📝 Docs preview for commit e61a157 at: https://636ccee0ff69b5781da53e61--fastapi.netlify.app

@github-actions
Copy link
Contributor

📝 Docs preview for commit e3e2305 at: https://6383671d27e2835f6df48cd9--fastapi.netlify.app

@github-actions
Copy link
Contributor

📝 Docs preview for commit 05200ca at: https://63836ff9da05a66ba14a4a60--fastapi.netlify.app

@github-actions
Copy link
Contributor

📝 Docs preview for commit bee76da at: https://639ce81bf6fa040074688124--fastapi.netlify.app

@github-actions
Copy link
Contributor

github-actions bot commented Jun 2, 2023

📝 Docs preview for commit e93ac54 at: https://64799e45640e0700d467febb--fastapi.netlify.app

@github-actions
Copy link
Contributor

github-actions bot commented Jun 2, 2023

📝 Docs preview for commit e93ac54 at: https://64799e4e2d520a6718a26603--fastapi.netlify.app

@tiangolo tiangolo added p4 and removed investigate labels Jan 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants