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

Drop unnecessary Blinker dependency #248

Merged
merged 1 commit into from
Apr 29, 2024
Merged

Drop unnecessary Blinker dependency #248

merged 1 commit into from
Apr 29, 2024

Conversation

jeffwidman
Copy link
Member

@jeffwidman jeffwidman commented Apr 24, 2024

Flask requires this, so it's a transitive dep, but it's not directly used within this toolbar at all.

We do directly import the other listed packages, so they should stay.

@jeffwidman
Copy link
Member Author

Oh interesting, Python 3.7 requires Blinker...

Probably newer Flasks require it, but Flask 2.2.0 doesn't... not worth tracking down, but essentially this PR is blocked until we drop Python 3.7 support.

=================================== FAILURES ===================================
________________________________ test_basic_app ________________________________

    def test_basic_app():
        app = load_app('basic_app')
>       index = app.get('/')

test/test_toolbar.py:14: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
.tox/py[37](https://github.com/pallets-eco/flask-debugtoolbar/actions/runs/8809300665/job/24179876713?pr=248#step:6:38)/lib/python3.7/site-packages/werkzeug/test.py:1141: in get
    return self.open(*args, **kw)
.tox/py37/lib/python3.7/site-packages/flask/testing.py:2[41](https://github.com/pallets-eco/flask-debugtoolbar/actions/runs/8809300665/job/24179876713?pr=248#step:6:42): in open
    follow_redirects=follow_redirects,
.tox/py37/lib/python3.7/site-packages/werkzeug/test.py:1095: in open
    response = self.run_wsgi_app(request.environ, buffered=buffered)
.tox/py37/lib/python3.7/site-packages/werkzeug/test.py:962: in run_wsgi_app
    rv = run_wsgi_app(self.application, environ, buffered=buffered)
.tox/py37/lib/python3.7/site-packages/werkzeug/test.py:12[43](https://github.com/pallets-eco/flask-debugtoolbar/actions/runs/8809300665/job/24179876713?pr=248#step:6:44): in run_wsgi_app
    app_rv = app(environ, start_response)
.tox/py37/lib/python3.7/site-packages/flask/app.py:2552: in __call__
    return self.wsgi_app(environ, start_response)
.tox/py37/lib/python3.7/site-packages/flask/app.py:2532: in wsgi_app
    response = self.handle_exception(e)
.tox/py37/lib/python3.7/site-packages/flask/app.py:2529: in wsgi_app
    response = self.full_dispatch_request()
.tox/py37/lib/python3.7/site-packages/flask/app.py:1825: in full_dispatch_request
    rv = self.handle_user_exception(e)
.tox/py37/lib/python3.7/site-packages/flask/app.py:1821: in full_dispatch_request
    rv = self.preprocess_request()
.tox/py37/lib/python3.7/site-packages/flask/app.py:2313: in preprocess_request
    rv = self.ensure_sync(before_func)()
.tox/py37/lib/python3.7/site-packages/flask_debugtoolbar/__init__.py:175: in process_request
    DebugToolbar(real_request, self.jinja_env))
.tox/py37/lib/python3.7/site-packages/flask_debugtoolbar/toolbar.py:23: in __init__
    self.create_panels()
.tox/py37/lib/python3.7/site-packages/flask_debugtoolbar/toolbar.py:34: in create_panels
    context=self.template_context)
.tox/py37/lib/python3.7/site-packages/flask_debugtoolbar/panels/template.py:37: in __init__
    template_rendered.connect(self._store_template_info)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <flask.signals._FakeSignal object at 0x7f2093568ed0>
args = (<bound method TemplateDebugPanel._store_template_info of <flask_debugtoolbar.panels.template.TemplateDebugPanel object at 0x7f20920ab8[50](https://github.com/pallets-eco/flask-debugtoolbar/actions/runs/8809300665/job/24179876713?pr=248#step:6:51)>>,)
kwargs = {}

    def _fail(self, *args: t.Any, **kwargs: t.Any) -> t.Any:
        raise RuntimeError(
            "Signalling support is unavailable because the blinker"
            " library is not installed."
>       ) from None
E       RuntimeError: Signalling support is unavailable because the blinker library is not installed.

Flask requires this, so it's a transitive dep, but it's not directly used within this toolbar at all.

We do directly import the other listed packages, so they should stay.
@jeffwidman
Copy link
Member Author

We dropped 3.7 support:

So this PR is unblocked... I've rebased it and it should be good to go.

@davidism davidism merged commit bfd4d85 into main Apr 29, 2024
10 checks passed
@davidism davidism deleted the drop-blinker-dep branch April 29, 2024 17:28
@jeffwidman
Copy link
Member Author

🤔 I wonder though... it seems the exception in #248 (comment) implies that we are directly relying on Blinker somehow... My guess is for signals, although I haven't tracked down whether FDBT is using signals all the time, or just in that particular test.

Regardless, now that we've dropped Python 3.7 support, the simpler thing might be to bump Flask to 2.3.0:

@davidism
Copy link
Member

I don't think we need any dependency except Flask itself. Declaring Flask's own dependencies here is redundant, and would presumably break anyway if Flask were to not depend on them anymore (not that it would do that).

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 14, 2024
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.

2 participants