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

fix(2.x): session get bind signature #1001

Closed

Conversation

dpgaspar
Copy link

@dpgaspar dpgaspar commented Sep 10, 2021

Revisiting this issue, patch for 2.x

The SignallingSession get_bind is being called without any parameters, this is probably due to the new SQLAlchemy proxied mechanism for registering scoped sessions

(None, None, None, None, False) {}

This is a simple fix that just uses the exact method signature from SQLAlchemy for get_bind of the current session

fixes: Possible issue on session get_bind #953

Note: Because of the CI drift between main and 2.x I'm posting my output from tox here:

=============================================== test session starts ===============================================
platform darwin -- Python 3.7.6, pytest-6.2.5, py-1.10.0, pluggy-1.0.0
cachedir: .tox/py37/.pytest_cache
rootdir: /Users/daniel/workarea/preset/flask-sqlalchemy, configfile: setup.cfg, testpaths: tests
collected 74 items

tests/test_basic_app.py ....                                                                                [  5%]
tests/test_binds.py ....                                                                                    [ 10%]
tests/test_commit_on_teardown.py ..                                                                         [ 13%]
tests/test_config.py ...................                                                                    [ 39%]
tests/test_meta_data.py ..                                                                                  [ 41%]
tests/test_model_class.py ....                                                                              [ 47%]
tests/test_pagination.py .....                                                                              [ 54%]
tests/test_query_class.py ...                                                                               [ 58%]
tests/test_query_property.py ....                                                                           [ 63%]
tests/test_regressions.py ....                                                                              [ 68%]
tests/test_sessions.py .....                                                                                [ 75%]
tests/test_signals.py ..                                                                                    [ 78%]
tests/test_sqlalchemy_includes.py .                                                                         [ 79%]
tests/test_table_name.py .............                                                                      [ 97%]
tests/test_utils.py ..                                                                                      [100%]

================================================ warnings summary =================================================
tests/test_commit_on_teardown.py::test_commit_on_success
tests/test_commit_on_teardown.py::test_roll_back_on_failure
  /Users/daniel/workarea/preset/flask-sqlalchemy/flask_sqlalchemy/__init__.py:903: DeprecationWarning: 'COMMIT_ON_TEARDOWN' is deprecated and will be removed in version 3.1. Call 'db.session.commit()'` directly instead.
    DeprecationWarning,

-- Docs: https://docs.pytest.org/en/stable/warnings.html
========================================= 74 passed, 2 warnings in 0.95s ==========================================
_____________________________________________________ summary _____________________________________________________
  py37: commands succeeded
  congratulations :)
 ~/w/p/flask-sqlalchemy  fix/get-session-bind-2x-v2 *2 ?2  tox -e py38               ok  14s  flask-sqlalchemy py
GLOB sdist-make: /Users/daniel/workarea/preset/flask-sqlalchemy/setup.py
py38 create: /Users/daniel/workarea/preset/flask-sqlalchemy/.tox/py38
py38 installdeps: pytest, coverage, blinker, mock
py38 inst: /Users/daniel/workarea/preset/flask-sqlalchemy/.tox/.tmp/package/1/Flask-SQLAlchemy-2.5.1.zip
py38 installed: attrs==21.2.0,blinker==1.4,click==8.0.1,coverage==5.5,Flask==2.0.1,Flask-SQLAlchemy @ file:///Users/daniel/workarea/preset/flask-sqlalchemy/.tox/.tmp/package/1/Flask-SQLAlchemy-2.5.1.zip,greenlet==1.1.1,iniconfig==1.1.1,itsdangerous==2.0.1,Jinja2==3.0.1,MarkupSafe==2.0.1,mock==4.0.3,packaging==21.0,pluggy==1.0.0,py==1.10.0,pyparsing==2.4.7,pytest==6.2.5,SQLAlchemy==1.4.23,toml==0.10.2,Werkzeug==2.0.1
py38 run-test-pre: PYTHONHASHSEED='1751660045'
py38 runtests: commands[0] | coverage run -p -m pytest --tb=short --basetemp=/Users/daniel/workarea/preset/flask-sqlalchemy/.tox/py38/tmp
=============================================== test session starts ===============================================
platform darwin -- Python 3.8.9, pytest-6.2.5, py-1.10.0, pluggy-1.0.0
cachedir: .tox/py38/.pytest_cache
rootdir: /Users/daniel/workarea/preset/flask-sqlalchemy, configfile: setup.cfg, testpaths: tests
collected 74 items

tests/test_basic_app.py ....                                                                                [  5%]
tests/test_binds.py ....                                                                                    [ 10%]
tests/test_commit_on_teardown.py ..                                                                         [ 13%]
tests/test_config.py ...................                                                                    [ 39%]
tests/test_meta_data.py ..                                                                                  [ 41%]
tests/test_model_class.py ....                                                                              [ 47%]
tests/test_pagination.py .....                                                                              [ 54%]
tests/test_query_class.py ...                                                                               [ 58%]
tests/test_query_property.py ....                                                                           [ 63%]
tests/test_regressions.py ....                                                                              [ 68%]
tests/test_sessions.py .....                                                                                [ 75%]
tests/test_signals.py ..                                                                                    [ 78%]
tests/test_sqlalchemy_includes.py .                                                                         [ 79%]
tests/test_table_name.py .............                                                                      [ 97%]
tests/test_utils.py ..                                                                                      [100%]

================================================ warnings summary =================================================
tests/test_commit_on_teardown.py::test_commit_on_success
tests/test_commit_on_teardown.py::test_roll_back_on_failure
  /Users/daniel/workarea/preset/flask-sqlalchemy/flask_sqlalchemy/__init__.py:899: DeprecationWarning: 'COMMIT_ON_TEARDOWN' is deprecated and will be removed in version 3.1. Call 'db.session.commit()'` directly instead.
    warnings.warn(

-- Docs: https://docs.pytest.org/en/stable/warnings.html
========================================= 74 passed, 2 warnings in 1.08s ==========================================
_____________________________________________________ summary _____________________________________________________
  py38: commands succeeded
  congratulations :)

Checklist:

  • Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
  • Add or update relevant docs, in the docs folder and in code.
  • Add an entry in CHANGES.rst summarizing the change and linking to the issue.
  • Add .. versionchanged:: entries in any relevant code docs.
  • Run pre-commit hooks and fix any issues.
  • Run pytest and tox, no tests failed.

@dpgaspar dpgaspar marked this pull request as ready for review September 10, 2021 09:55
@dpgaspar
Copy link
Author

@davidism gentle ping

@davidism
Copy link
Member

I'm sorry, I will not have time to review this in the near future

@kanetkarster
Copy link

Any word on this or anything I can do (either on the technical or donation side) to help get this through/in a new release?

We have airflow in a virtual environment with some of our code and we'd need this get_bind change to update our sqlalchemy version.

@luistm

This comment has been minimized.

@pallets-eco pallets-eco deleted a comment from kaxil Dec 23, 2021
@pallets-eco pallets-eco locked and limited conversation to collaborators Dec 23, 2021
@davidism
Copy link
Member

Fixed in #1087

@davidism davidism closed this Sep 18, 2022
@ashb ashb deleted the fix/get-session-bind-2x-v2 branch January 9, 2023 09:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants