search

pallets-eco / flask-debugtoolbar

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

Review flask.dispatch_request for possible tweaks #150

Closed jeffwidman closed 4 years ago

jeffwidman commented 4 years ago

Our dispatch_request() was a modified version of flask.dispatch_request(), but that fork happened years ago.

Now would be a good time to review how they wire it to see if there are any additional improvements/tweaks that should be pulled in: https://github.com/pallets/flask/blob/a3e4395a42574386a15ef6ca1afa83ff9787608f/src/flask/app.py#L1907-L1946

mattaw commented 4 years ago

Having reviewed the code in flask-debugtoolbar and that in the current version of Flask I can't see any functional difference (except wiring in debugtoolbar process_view). However, in the process of exploring what it was doing I am concerned that the debugtoolbar testing through tox passes just fine if I comment out the monkeypatch completely so I will try to improve that.

jeffwidman commented 4 years ago

Thanks for looking into this @mattaw!

Sounds great, then let's close this. I didn't check the code since you already did, but I did check git blame on that section of Flask code and you're right, most of it hasn't been modified in 9-10 years!

I opened https://github.com/flask-debugtoolbar/flask-debugtoolbar/issues/151 to track the test improvement.