-
-
Notifications
You must be signed in to change notification settings - Fork 900
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
Support SQLAlchemy 2's DeclarativeBase
and MappedAsDataclass
#1140
Comments
I looked into both Flask-SQLAlchemy allows passing either a plain class or a declarative base class as flask-sqlalchemy/src/flask_sqlalchemy/extension.py Lines 453 to 457 in 6605f9a
Since you're using SQLAlchemy 2's new Our code for setting table names and supporting joined-table inheritance requires running before and after flask-sqlalchemy/src/flask_sqlalchemy/model.py Lines 108 to 119 in 6605f9a
I'm optimistic that there's some way to refactor our code to provide support for both |
mapped_column
and dataclasses-like models via MappedAsDataclass
DeclarativeBase
and MappedAsDeclarative
DeclarativeBase
and MappedAsDeclarative
DeclarativeBase
and MappedAsDataclass
I understand what and why is happening but, unfortunately, have no useful input on how. Thanks for looking into this. |
Ciao! I had a look at the previous issues and this seems to me the first time the compatibility with SQLAlchemy 2.0 is discussed, am I right? This seems like a pretty advanced topic to start contributing to, but I'd be willing to help out with some guidance. |
Not sure that it matters, but this is the first time compatibility with I don't have any guidance to give besides what I've written above. I don't know how to solve this any more than you do, I would be doing the same investigation. |
This comment was marked as off-topic.
This comment was marked as off-topic.
I can take a look at this if that works for you @davidism @baggiponte (though not suggesting I'll definitely make the the required changes, in case others have time/appetite) |
No need to ask to work on something, go for it. |
Query result typehints (my selfish concern) actually work fine when upgrading to the latest sqlalchemy 2.0 beta without using That being said, a few things:
|
Thanks @jdimmerman for the investigation, very helpful.
In flask_sqlalchemy/extension.py:make_declarative_base, I changed the if to:
Then in my own code, I inherited from
And specified that as the model class:
And that enabled me to use the Mapped attributes. See full change in: I'll see if I can make the change to extension.py inside flask-sqlalchemy itself with tests. I haven't tried data classes however, don't know if that'll work too. |
Okay, so I've done more investigation of my approach inside the flask-sqlalchemy codebase itself. This comment may be a bit long as I share my findings. First, in order to understand the metaclass/subclass distinction better, I made a diagram of relevant SQLAlchemy classes: I realized that my above approach was also compatible with using With that small change, I could use Flask-SQLAlchemy features like get_or_404. However, I couldn't use the functionality provided by the mixins in model.py, like table name generation, bind keys, and custom repr. All of those are implemented as metaclass mixins, which isn't compatible with the new subclass approach. The SQLAlchemy2.0 docs suggest 2 main approaches for using mixins with the DeclarativeBase:
A third possibility mentioned in some GitHub threads is to use I tried both approaches #1 and #2 but it felt weird for Flask-SQLAlchemy to create its own Base, so I settled on #1 as the better of those options. Here's what it looks like: I basically combined all the previous mixins into a single So, in conclusion:
I'm happy to send the PR for the minor change if that still seems useful/desired. |
I've made another branch with a different approach: In this one, the dev only needs to specify the model class of DeclarativeBase:
Then, in the flask-sqlalchemy code, it dynamically creates a Base class that mixes in a subclass version of each mixin:
This code passes at least basic tests for the mixin functionality - bind key, name, repr, etc. And if someone wanted just some of those, they could create their own Base class. This seems fairly promising as an approach. |
Your diagram is a much more organzied form of what I kept in my head when working on all this. 😅 Add the timing of when I'm not tied to using any specific solution. metaclasses, mixins, or |
Thanks @davidism for the reply, glad this seems promising. I realized after posting that last comment that the original request was from @tadams42 wanting MappedAsDataclass and my suggested interface didn't account for that. In this one, the developer writes the BaseModel themselves, such as:
And then flask-sqlalchemy constructs db.Model using the bases to create the mixed in version:
I've parameterized the test to check that it works with various combos of DeclarativeBase, DeclarativeBaseNoMeta, and MappedAsDataclass. I'll keep running tests and also look into the timing, and send a PR based on this approach, since it seems like the best so far. |
I am currently trying to figure out a way to define my model that doesn't depend on Flask-SQLAlchemy. I have tried The error I have been getting is
And should I make another help thread about my specific case, or is my issue basically this issue? It sounds like it's being worked on but not there yet? |
That error is coming from SQLAlchemy ( I sent out a PR last week with changes to support a Base like that, so you could try bringing in my changes and see if they work for you. |
I did also get metaclass errors with some things slightly different. I'm up for trying your changes, but I will almost certainly need some hand-holding. |
To try the branch, first make sure Flask-SQLalchemy is uninstalled in your environment and then install it from the branch. Here's how I did that for a project using pip:
Then I replaced flask-sqlalchemy in my requirements.txt with this:
And ran:
You can also directly install with:
Here's a sample app I just tested using the same base classes you're looking for - this diff shows my diff between SQLAlchemy 1.4 and 2.0: |
Thank you! I was able to install your branch and now I can it least run |
Installed third version of @pamelafox repo (
After setting up minimal flask app I got suggestions out of the box, except for Overall, much much more pleasurable to see reduced white color in vscode. Great job! Hope this gets into release soon! |
Finally got time to try all this out. Fully converted small Flask app with All in all, everything works perfectly! 🎊 🎉 @pamelafox Awesome work, thank you! |
I've been playing with
mapped_column
andMappedAsDataclass
(new stuff in SQLAlchemy 2.x):Declarative Table with mapped_column()
Declarative Dataclass Mapping
Example:
This rather verbose declaration of models gives us some nice things:
mypy
andPyRight
static typechecking.__init__
First try at using my
BaseModel
withFlask-SQLAlchemy
gets me into trouble withmetaclass
inheritance:which is fair enough...
I did try few things from Advanced Customization but so far came empty handed.
Can Flask-SQLAlchemy support this "new" declarative models?
Should it?
Maybe avoid this problem by somehow replacing metaclass magic with
__init_subclass__
fromPEP 487 - Simpler customisation of class creation
The text was updated successfully, but these errors were encountered: