pallets-eco / flask-debugtoolbar

A toolbar overlay for debugging Flask applications
https://flask-debugtoolbar.readthedocs.io
BSD 3-Clause "New" or "Revised" License
952 stars 146 forks source link

updated to work with flask 2.2+ #183

Closed christopherpickering closed 2 years ago

christopherpickering commented 2 years ago

Replaced _request_ctx_stack.top with request_ctx per https://github.com/pallets/flask/pull/4682/files

nickjj commented 2 years ago

Hi,

I'm using the currently released version of DTB in a Flask 2.2+ app and it's working here. Did you encounter an error?

christopherpickering commented 2 years ago

@nickjj, hey, no, no errors, just the depreciation warning that in flask 2.3 the _request_ctx_stack will be removed.

nickjj commented 2 years ago

Ah right.

This project currently has Flask >= 0.8 defined. I do like the idea of removing the deprecation warning by using the "new" way for Flask 2.3 but we should figure out a solution that still works for versions < 2.3. I think you could detect the version of Flask and import either one based on the version and use as to ensure the import name is the same for both so the usage can remain as req = request_ctx.request.

What do you think?

christopherpickering commented 2 years ago

Yeah, that's an idea. From the flask PR, they said this was existing all along, but I'm not sure what version of flask it was really added.

We could something like this: int(flask.version.split(".")[0]) < 2 and int(flask.version.split(".")[1]) < 2

Probably there is a cleaner way?

nickjj commented 2 years ago

I didn't fully test this but if you put this into a Python interpreter it works. The packaging library is part of Python's standard library:

import flask

from packaging import version

version.parse(flask.__version__) >= version.parse("2.2.0")

That condition is what you could do to require a specific package from Flask.

christopherpickering commented 2 years ago

How this look @nickjj ?

The tests run fine w/ tox, but not the stylecheck. It looks like the tox file needs updated?

jeffwidman commented 2 years ago

I filed https://github.com/flask-debugtoolbar/flask-debugtoolbar/issues/184 to track bumping our version of Flask... I'm very open to doing so if /when we need it. But as @nickjj pointed out here, for this PR checking the version is a clean-backwards compatible method, so 👍 for that here.

nickjj commented 2 years ago

I'm generally ok with it as a concept. I'm not loving repeating that version check twice but this also feels like a spot where if it were refactored and made into a general version compare function it might make things less readable in the end.

Do you want to wait for the author of the PR to potentially remove that 1 empty line before merging it @jeffwidman?

jeffwidman commented 2 years ago

Thanks again!