Remove Flask-SQLAlchemy
#9989
Remove Flask-SQLAlchemy
#9989
Conversation
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
|
Oh, looks like the way Flask-SQLAlchemy is packaged breaks some of the logic for dealing with non-types dependencies in |
|
I will take a look later, there are two unrelated problems. |
|
The pyright stuff is very strange. It's erroring out on some types in stubs/SQLAlchemy. But you haven't changed anything in that directory! |
This comment has been minimized.
This comment has been minimized.
| # Requires a version of flask with a `py.typed` file | ||
| requires = ["Flask>=2.0.0", "types-Flask-SQLAlchemy"] | ||
| # Requires versions of flask/Flask-SQLAlchemy with `py.typed` files | ||
| requires = ["Flask>=2.0.0", "Flask-SQLAlchemy>=3.0.1"] |
There was a problem hiding this comment.
I figured it out. Because you're specifying Flask-SQLAlchemy as a dependency of types-Flask-Migrate, that then pulls in SQLAlchemy as a transitive dependency of types-Flask-Migrate (because SQLAlchemy is a dependency of Flask-SQLAlchemy). That means pip now install SQLAlchemy in CI before pyright tests any of the stubs and, because it's unpinned, pip installs the latest version of SQLAlchemy available. Because SQLAlchemy>=2 has inline types and a py.typed file, that means that pyright is now pointing out the inconsistencies between our stubs for SQLAlchemy<2 and the inline types it sees the site-packages directory for SQLAlchemy>=2
There was a problem hiding this comment.
How to fix this is another question. I'm open to ideas ://
There was a problem hiding this comment.
We could just add the missing generics. So that the stubs are on par enough with the 2.0 version for this PR to pass.
There was a problem hiding this comment.
We could just add the missing generics. So that the stubs are on par enough with the 2.0 version for this PR to pass.
It feels like a somewhat fragile solution -- if SQLAlchemy makes more stuff generic on the 2.0 branch that isn't generic in our stubs for SQLAlchemy 1.0, and they then make a release, our CI will break unexpectedly
There was a problem hiding this comment.
Then I think just waiting to remove them together, given their co-dependencies, just makes the most sense.
|
An easy way of solving the pyright issue, given our Sqlalchemy stubs are already marked as obsolete, would be to just wait until we're ready to remove those stubs, and then remove them both together |
Needed for python/typeshed#9989. There are other blockers on that PR at the moment, but we will *eventually* want to have our stubs for `Flask-Migrate` declare a dependency on `Flask-SQLAlchemy`, even if we have to wait a little bit before we're able to do that.
Needed for python/typeshed#9989. There are other blockers on that PR at the moment, but we will *eventually* want to have our stubs for `Flask-Migrate` declare a dependency on `Flask-SQLAlchemy`, even if we have to wait a little bit before we're able to do that.
This comment has been minimized.
This comment has been minimized.
|
Diff from mypy_primer, showing the effect of this PR on open source code: kornia (https://github.com/kornia/kornia) got 3.52x faster (7.1s -> 2.0s)
|
|
Closing, as this can only be done at the same time as removing our (There's also a ton of merge conflicts now!) |
Closes #8885