Use MyPy support for SQLAlchemy

[kaxil]

Body

https://docs.sqlalchemy.org/en/14/orm/extensions/mypy.html describes how type checking can be used with SQLAlchemy.

This is possible after we bump the SQLAlchemy version to 1.4+

[uranusjr]

Except we're still on 1.3 😔

[kaxil]

Yeah :( Which dep is causing us to stay on <1.4, I can't remember anymore

[kaxil]

#16630

Flask-Appbuilder ... 😢

[potiuk]

Ehhhh

[kaxil]

Actually, it looks like it is because of pallets-eco/flask-sqlalchemy#1001 that FAB has to still use <1.4. FAB Issue: dpgaspar/Flask-AppBuilder#1710

[potiuk]

[kaxil]

Very apt XKCD 😄

[bmckallagat-os]

@kaxil I noticed they deleted your comment and locked the convo... petition to start an opensource war?

[potiuk]

@kaxil I noticed they deleted your comment and locked the convo... petition to start an opensource war?

I started a few already, some of them with success .... some of them ... not yet. Maybe time for another one ;)

[bmckallagat-os]

@kaxil I noticed they deleted your comment and locked the convo... petition to start an opensource war?

I started a few already, some of them with success .... some of them ... not yet. Maybe time for another one ;)

Please, use that Apache clout 🤣

[uranusjr]

I took a look at the code path and tried an alternative fix that does not need to involve the Pallets team (since they do not seem to be interested in supporting this): dpgaspar/Flask-AppBuilder#1783

[boredland]

so isn't this done now via dpgaspar/Flask-AppBuilder#1786 ?

[potiuk]

It's in https://pypi.org/project/Flask-AppBuilder/3.4.4rc1/ - so as soon as it's released we will be able to switch to it.

[boredland]

perhaps I am misunderstanding something, but to me it looks like somewhere (didn't find where) there seems to be a hardcoded prefix 'postgres://' for connections. that surely will break after the 1.4. upgrade and would have to be 'postgresql://':

[zzzeek]

hi just an FYI, the SQLAlchemy mypy plugin is not going anywhere but in the 2.0 series of SQLAlchemy we hope to have a completely plugin-less approach to typing in place and the mypy plugin itself should become legacy. bugs within the mypy plugin that don't have an easy fix we will likely just "wontfix". so support the plugin sure but don't get too reliant on it.

[potiuk]

perhaps I am misunderstanding something, but to me it looks like somewhere (didn't find where) there seems to be a hardcoded prefix 'postgres://' for connections. that surely will break after the 1.4. upgrade and would have to be 'postgresql://':

Not really. This is "Airflow" connection to connect to Postgres via "postgres provider" - it's not even an SQLAlchemy connection - we are using DBApi there and Postgres Provider exposes plain old "SQL" interface to Postgres Provider (It would be next to impossible to have generic SQLAlchemy ORM interface).

SQLAlchemy is only used for Airlfow's internal meta-data as part of the application.

[saulbein]

dpgaspar/Flask-AppBuilder/releases/tag/v3.4.4 the fix has now been released - it would be nice to be able to update sqlalchemy to 1.4 since we have some other dependencies that we would like to update and they require 1.4.

[potiuk]

dpgaspar/Flask-AppBuilder/releases/tag/v3.4.4 the fix has now been released - it would be nice to be able to update sqlalchemy to 1.4 since we have some other dependencies that we would like to update and they require 1.4.

We did already - but currently we are dealing with the aftermath of it - there are couple of things that broke even our PRs and main builds and we are a bit scrambling to get the full SQLAlchemy 1.4 compatibility (and FAB 3.4.4 compatibility) in main. This caused persistent failures as well as intermittent flaky tests in our CI builds.

There are many issues: #20874 #19294 #21294 #21296 #21272

Until those are fixed and our main build is stabilized, there is not going to be any work on this one.

BTW. If you want you are most welcome to help with any of the above (see the comments to find out how and if you can help) or even attempt to work on adding MyPy support n Parallel.

[potiuk]

@saulbein - maybe you would like to add support for MyPY - seems that most of the problems with SQLAlchemy 1.4 have been addresse,d you could take care about it ?

[bmckallagat-os]

@potiuk Are there any other related issues besides #20874 #19294 #21294 #21296 #21272?

Happy to help but those seem to be all resolved or taken.

[potiuk]

go ahead. There is still an issue with going beyond SQLAlchemy 4.1.9 (but It should not clash with mypy support)

[saulbein]

@potiuk I would like to help, but sadly I don't know much (if at all) about Flask-AppBuilder and can't allocate enough time to learn with other work 😞. I'll see if I can at least contribute some typing.

[bmckallagat-os]

@potiuk I apologize, I just saw this now. I'm going on vacation until the 7th and I will try to tackle this after that time. If anyone wants to assign to themselves, please feel free. Otherwise I will make sure to get this done then.

[potiuk]

How about now :) ?

[bmckallagat-os]

@potiuk Finally getting back to this. Just wanted to note that the docs listed in the description state that there will be native type checking without the use of the plugin when 2.0 is released and it is now considered a legacy library.

I'll just add that change however and add a note in the setup.cfg.

[potiuk]

Cool

[eladkal]

https://docs.sqlalchemy.org/en/20/orm/extensions/mypy.html

However, SQLAlchemy 2.0, when released, will feature an all new typing system for ORM Declarative models that removes the need for the Mypy plugin and delivers much more consistent behavior with generally superior capabilities.

I guess we should wait till after we bump to 2.0 (after we completely remove 1.4)? #28723