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

✨ Raise a more clear error when a type is not valid #425

Merged
merged 11 commits into from
Oct 23, 2023

Conversation

ddanier
Copy link
Contributor

@ddanier ddanier commented Aug 29, 2022

Currently using Dict[...], List[...] or Union[...] will break with an error stating that issubclass() cannot be used for non types. This is something users won't understand and I think SQLModel should handle this in a better way.

PR will fix the issue and contains a test to validate the code was broken before and is now fixed. The normal ValueError(f"The field {field.name} has no matching SQLAlchemy type") will then be raised.

@github-actions
Copy link

📝 Docs preview for commit 7967bcb at: https://630cda8afbefa94f41c70df1--sqlmodel.netlify.app

@codecov
Copy link

codecov bot commented Aug 29, 2022

Codecov Report

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

Comparison is base (ea79c47) 98.49% compared to head (55c10b5) 97.73%.
Report is 50 commits behind head on main.

❗ Current head 55c10b5 differs from pull request most recent head 4e376df. Consider uploading reports for the commit 4e376df to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #425      +/-   ##
==========================================
- Coverage   98.49%   97.73%   -0.77%     
==========================================
  Files         185      187       +2     
  Lines        5856     6212     +356     
==========================================
+ Hits         5768     6071     +303     
- Misses         88      141      +53     
Files Coverage Δ
tests/test_get_sqlachemy_type.py 100.00% <100.00%> (ø)
sqlmodel/main.py 84.72% <67.56%> (ø)

... and 2 files with indirect coverage changes

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

@github-actions
Copy link

📝 Docs preview for commit 55c10b5 at: https://630cfc073e4a7000b12222dd--sqlmodel.netlify.app

@tiangolo tiangolo added the bug Something isn't working label Oct 22, 2023
@tiangolo tiangolo changed the title Fix get_sqlachemy_type() not checking for the ModelField.type_ to be a scalar type first ✨ Raise a more clear error when a type is not valid Oct 23, 2023
@tiangolo tiangolo added feature New feature or request and removed bug Something isn't working labels Oct 23, 2023
@tiangolo
Copy link
Member

Thanks! I updated the tests to use models, as that would be a realistic use case, then I fixed a small bug in the implementation detected by that.

This will be available in the next release. 🤓

@tiangolo tiangolo merged commit 840fd08 into fastapi:main Oct 23, 2023
10 checks passed
@ddanier ddanier deleted the fix-get_sqlachemy_type branch October 23, 2023 08:26
@ddanier
Copy link
Contributor Author

ddanier commented Oct 23, 2023

Thx for merging this! 🥳

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

Successfully merging this pull request may close these issues.

2 participants