-
-
Notifications
You must be signed in to change notification settings - Fork 719
🐛 Fix enum type checks ordering in get_sqlalchemy_type
#442
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
Conversation
When enum subclassing/mix-in[1] is used, especially to ensure compatibility and produce correct OpenAPI specifications with FastAPI[2], SQLModel on its side doesn't map to the correct SQLAlchemy type. e.g.: * `class Foo(Enum)` will work but won't allow a proper OpenAPI spec. to be generated. * `class Foo(str, Enum)` will fix the previous situation, but prevents SQLModel to read and map the object from the DB (due to validation errors). Changing ordering of checks to ensure that Enum subclasses are tested first. [1] https://docs.python.org/3/library/enum.html#restricted-enum-subclassing [2] https://fastapi.tiangolo.com/sq/tutorial/path-params/#create-an-enum-class
📝 Docs preview for commit a6d9568 at: https://631a68e284fe1d20404a6714--sqlmodel.netlify.app |
Codecov Report
@@ Coverage Diff @@
## main #442 +/- ##
=======================================
Coverage 97.76% 97.76%
=======================================
Files 187 187
Lines 6268 6268
=======================================
Hits 6128 6128
Misses 140 140
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Hmm, seeing that I'm not the only one to observe this:
@tiangolo any thoughts? |
This makes sense. It may need to be rebased with PR #425. If PR #436 is merged, you'll be able to specify the SQL type explicitly whenever you want. Then PR #366 will become obsolete. It's still worth maintaining/optimizing |
Indeed, just seen you proposal (#436), sounds really nice. |
Thank you! As I can't push to this branch, I included your commits in a new branch in a new PR here: #669 Given that, I'll close this one. Thanks for the work! 🙇 |
When enum subclassing/mix-in[1] is used, especially to ensure compatibility and produce correct OpenAPI specifications with FastAPI[2], SQLModel on its side doesn't map to the correct SQLAlchemy type.
e.g.:
class Foo(Enum)
will work but won't allow a proper OpenAPI spec. to be generated.class Foo(str, Enum)
will fix the previous situation, but prevents SQLModel to read and map the object from the DB (due to validation errors).Changing ordering of checks to ensure that Enum subclasses are tested first.
[1] https://docs.python.org/3/library/enum.html#restricted-enum-subclassing
[2] https://fastapi.tiangolo.com/sq/tutorial/path-params/#create-an-enum-class