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

SQLAlchemy Session __enter__ and __exit__ methods not being noticed. #2088

Closed
howbazaar opened this issue Nov 18, 2021 · 7 comments
Closed
Labels
bug Something isn't working typestub Issue relating to our bundled type stubs waiting for upstream Waiting for upstream to release a fix

Comments

@howbazaar
Copy link

howbazaar commented Nov 18, 2021

Environment data

  • Language Server version: 2021.11.1
  • OS and version: linux x64
  • Python version: cpython 3.8
  • python.analysis.indexing: undefined
  • python.analysis.typeCheckingMode: basic

Expected behaviour

We have a session builder being used, and a method new_session:

from sqlalchemy.orm import Session
#...
    @staticmethod
    def new_session() -> Session:
        dbm = DBManagerBuilder()
        Session = dbm().get_session()
        return Session()

All places in the code where we call this method should be error free:

    with rdb.new_session() as session:

Actual behaviour

All places in the code where we call this method

    with rdb.new_session() as session:

gets red squiggly lines under the rdb.new_session()

Object of type "Session" cannot be used with "with" because it does not implement __enter__PylancereportGeneralTypeIssues
Object of type "Session" cannot be used with "with" because it does not implement __exit__PylancereportGeneralTypeIssues
(method) new_session: () -> Session

Following the Session definition through, the __enter__ and __exit__ methods are clearly there.

Logs

Python Language Server Log

XXX 'Please paste the output from your clipboard'
typeshed-fallback/stdlib/logging/__init__.pyi [fs read 0ms] (28ms)
[FG] binding: /home/tim/.vscode/extensions/ms-python.vscode-pylance-2021.11.1/dist/typeshed-fallback/stdlib/logging/__init__.pyi (6ms)
[FG] parsing: /home/tim/.vscode/extensions/ms-python.vscode-pylance-2021.11.1/dist/bundled/stubs/sqlalchemy/orm/__init__.pyi [fs read 0ms] (2ms)
[FG] binding: /home/tim/.vscode/extensions/ms-python.vscode-pylance-2021.11.1/dist/bundled/stubs/sqlalchemy/orm/__init__.pyi (1ms)
[FG] parsing: /home/tim/.vscode/extensions/ms-python.vscode-pylance-2021.11.1/dist/bundled/stubs/sqlalchemy/orm/session.pyi [fs read 1ms] (13ms)
[FG] binding: /home/tim/.vscode/extensions/ms-python.vscode-pylance-2021.11.1/dist/bundled/stubs/sqlalchemy/orm/session.pyi (3ms)
Background analysis message: getDiagnosticsForRange
Background analysis message: getDiagnosticsForRange

I'm guessing that the stubs being bundled don't specify the enter and exit methods.

@erictraut
Copy link
Contributor

Yes, your diagnosis is correct. The bundled stubs don't include __enter__ or __exit__ methods in the Session class. If you click on Session and choose "Go to declaration", you can see the type information that is used here.

@erictraut
Copy link
Contributor

I'll also note that the latest published version of sqlalchemy-stubs also doesn't include these methods. Perhaps it was a recent addition to the library?

@erictraut
Copy link
Contributor

Maintaining compatibility between a library and the corresponding stubs presents a challenge for the Python ecosystem. Our preference and recommendation is that library maintainers include type information (in the form of inlined types or type stubs) as part of their library package. This is the best way to prevent incompatibilities like this. Thankfully, we're seeing that many libraries are now directly incorporating type information!

It helps if consumers of a library express their desire for type information to be included. If this is something you'd like to see for sqlalchemy, perhaps you'd be willing to file an enhancement request in the sqlalchemy repo?

@howbazaar
Copy link
Author

@erictraut
Copy link
Contributor

erictraut commented Nov 18, 2021

I think pylance currently bundles the stubs from sqlalchemy-stubs. @judej, could you confirm? If so, then the best way to address this in the short term is to submit a PR to the sqlalchemy-stubs repo and then incorporate the updated stubs in the next release of pylance.

Longer term, the best answer is for sqlalchemy to ship type information as part of their package.

@howbazaar
Copy link
Author

Have opened dropbox/sqlalchemy-stubs#231

@judej judej added typestub Issue relating to our bundled type stubs waiting for upstream Waiting for upstream to release a fix labels Nov 18, 2021
@github-actions github-actions bot removed the triage label Nov 18, 2021
@gramster gramster added the bug Something isn't working label Feb 24, 2022
@github-actions github-actions bot removed the triage label Feb 24, 2022
@debonte
Copy link
Contributor

debonte commented Apr 12, 2023

Triage: I think we should close this:

  1. I don't see any SQLAlchemy stubs bundled in Pylance.
  2. The PR to fix sqlalchemy-stubs has been open and unmerged for 17 months.
  3. SQLAlchemy 2.x has better typing support.

@judej judej closed this as completed Apr 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working typestub Issue relating to our bundled type stubs waiting for upstream Waiting for upstream to release a fix
Projects
None yet
Development

No branches or pull requests

6 participants