Skip to content
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

Asynchronous sessionmaker initialization type error #155

Closed
r4rdsn opened this issue Aug 20, 2021 · 13 comments · Fixed by #157
Closed

Asynchronous sessionmaker initialization type error #155

r4rdsn opened this issue Aug 20, 2021 · 13 comments · Fixed by #157
Labels
bug Something isn't working

Comments

@r4rdsn
Copy link

r4rdsn commented Aug 20, 2021

Describe the bug
mypy fails on asynchronous sessionmaker initialization.

Expected behavior
No errors.

To Reproduce
Please try to provide a Minimal, Complete, and Verifiable example.
See also Reporting Bugs on the website, and some example issues.

Please do not use Flask-SQLAlchemy or any other third-party extensions or dependencies in test cases. The test case must illustrate the problem without using any third party SQLAlchemy extensions. Otherwise, please report the bug to those projects first.

import os

from sqlalchemy.ext.asyncio import AsyncSession
from sqlalchemy.ext.asyncio import create_async_engine
from sqlalchemy.orm import sessionmaker

engine = create_async_engine(os.getenv("DATABASE_URL"))
async_session = sessionmaker(engine, class_=AsyncSession)

Error

session.py:8: error: Value of type variable "_TSessionMakerType" of "sessionmaker" cannot be "AsyncSession"
Found 1 error in 1 file (checked 1 source file)

Versions.

  • OS: Linux
  • Python: 3.9.6
  • SQLAlchemy: 1.4.23
  • mypy: 0.910
  • SQLAlchemy2-stubs: v0.0.2a9

Additional context
The problem appeared on the latest release of sqlalchemy2-stubs and was not present on v0.0.2a8.

Have a nice day!

@r4rdsn r4rdsn added the requires triage New issue that requires categorization label Aug 20, 2021
@CaselIT CaselIT added bug Something isn't working and removed requires triage New issue that requires categorization labels Aug 20, 2021
@CaselIT
Copy link
Member

CaselIT commented Aug 20, 2021

Thanks for the report

@joaosr
Copy link

joaosr commented Aug 20, 2021

Same happening for me.

@CaselIT
Copy link
Member

CaselIT commented Aug 20, 2021

released as v0.0.2a10

@mjpieters
Copy link
Contributor

mjpieters commented Sep 23, 2022

This appears to have regressed? I'm using version 0.0.2a27, and the example in this issue now produces:

Argument of type "Type[AsyncSession]" cannot be assigned to parameter "class_" of type "Type[_TSessionMakerType@sessionmaker]" in function "__init__"
  Type "Type[AsyncSession]" cannot be assigned to type "Type[_TSessionMakerType@sessionmaker]"

when using Pyright.

I noticed that almost the same error message appears when using sqlalchemy.create_engine() and sqlalchemy.orm.Session:

Argument of type "Type[Session]" cannot be assigned to parameter "class_" of type "Type[_TSessionMakerType@sessionmaker]" in function "__init__"
  Type "Type[Session]" cannot be assigned to type "Type[_TSessionMakerType@sessionmaker]"

@zzzeek
Copy link
Member

zzzeek commented Sep 23, 2022

@mjpieters I am hoping 2.0 betas are very soon. that's where these issues if you have them will be permanently fixed so i hope you have time to test a bit.

@mjpieters
Copy link
Contributor

Yeah, I can't find a release of sqlalchemy2-stubs that actually makes the example work, so it's probably a pyright incompatibility somewhere.

@CaselIT
Copy link
Member

CaselIT commented Sep 23, 2022

If you want to send a PR to fix things until v2 is out we can merge and release fairly quickly

@mjpieters
Copy link
Contributor

mjpieters commented Oct 11, 2022

If you want to send a PR to fix things until v2 is out we can merge and release fairly quickly

I traced this down to pyright not accepting that AsyncSession implements the TypingAsyncSessionProtocol type, because the signature of no_autoflush doesn't match:

Expression of type "Type[AsyncSession]" cannot be assigned to declared type "TypingAsyncSessionProtocol"
  "AsyncSession" is incompatible with protocol "TypingAsyncSessionProtocol"
  Type "Type[AsyncSession]" cannot be assigned to type "TypingAsyncSessionProtocol"
    "no_autoflush" is an incompatible type
      Type "(self: _TSessionNoIoTypingCommon@no_autoflush) -> ContextManager" cannot be assigned to type "(self: _TSharedSessionProtocol@no_autoflush) -> ContextManager"
        Parameter 1: type "_TSharedSessionProtocol@no_autoflush" cannot be assigned to type "_TSessionNoIoTypingCommon@no_autoflush"
          Type "_TSharedSessionProtocol@no_autoflush" cannot be assigned to type "_SessionNoIoTypingCommon[Unknown]"
        Function return type "ContextManager" is incompatible with type "ContextManager"
          "contextlib.AbstractContextManager" is incompatible with "contextlib.AbstractContextManager"

The issue could trivially be fixed if PEP 673 typing.Self / typing_extensions.Self was used there, but mypy doesn't yet support PEP 673. :-( I have not yet figured out what exactly is different between the context manager declarations or why using def no_autoflush(self) -> ContextManager[Self]: ... fixes the issue.

However, I think I can at least make this fail for mypy as well with:

from typing import TYPE_CHECKING, Type

from sqlalchemy.ext.asyncio import async_scoped_session
from sqlalchemy.ext.asyncio import AsyncSession
from sqlalchemy.ext.asyncio import create_async_engine
from sqlalchemy.orm import sessionmaker

if TYPE_CHECKING:
    from sqlalchemy.ext.asyncio.session import TypingAsyncSessionProtocol

engine = create_async_engine(...)
SessionType: Type[TypingAsyncSessionProtocol] = AsyncSession
SM = sessionmaker(engine, class_=SessionType)

async_session = AsyncSession(engine)

as_session = async_scoped_session(SM, current_task)

This is an update to a single test/files/async_stuff.py line, pulling out the class_ argument into a separate type variable. Pyright / Pylance will complain about the SessionType assignment and again about passing SM on to async_scoped_session(), while MyPy only complains about the latter:

.../test/files/async_stuff.py:19: error: Argument 1 to "async_scope..._" has type "Callable[[KwArg(Any)], _AsyncSessionProtocol]"

@mjpieters
Copy link
Contributor

mjpieters commented Oct 11, 2022

Right, so the simplest fix is to not use bound typevars for methods producing a contextmanager; for both _SharedSessionProtocol and _SessionNoIoTypingCommon use:

    def no_autoflush(self: _M) -> ContextManager[_M]: ...

It's essentially the same meaning; whatever no_autoflush() returns, is a ContextManager that'll produce self when you call __enter__ on it, and it won't affect the implementation as the stub is separate.

I was, however, wrong about my assessment that I could make mypy fail on the same issue; the error it produces when you use the Type[TypingAsyncSessionProtocol] annotation sticks even when Pylance is now happy with the annotations.

@mjpieters
Copy link
Contributor

I've traced this to a Pyright issue with @property and ContextManager: microsoft/pyright#4034

@CaselIT CaselIT reopened this Oct 12, 2022
@CaselIT
Copy link
Member

CaselIT commented Oct 12, 2022

Thanks for debugging this. If you could provide a PR with the fix it would be great!

@mjpieters
Copy link
Contributor

Thanks for debugging this. If you could provide a PR with the fix it would be great!

It's an acknowledged pyright bug; it's in their court. The type definition is correct, the type checker is not, so no PR needed!

@CaselIT
Copy link
Member

CaselIT commented Oct 12, 2022

oh ok, good to know! thanks for looking into it! We can close then!

@CaselIT CaselIT closed this as completed Oct 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants