Skip to content

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
Contributor

@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
Contributor

@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 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
@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

2 participants