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

flask run is not compatible with werkzeug middlewares #3256

Closed
killthekitten opened this issue Jun 9, 2019 · 6 comments
Closed

flask run is not compatible with werkzeug middlewares #3256

killthekitten opened this issue Jun 9, 2019 · 6 comments

Comments

@killthekitten
Copy link

Expected Behavior

When using DispatcherMiddleware to combine multiple flask apps, I expect to be able to run the app with the following command:

FLASK_APP=main:app flask run
from flask import Flask
from werkzeug.middleware.dispatcher import DispatcherMiddleware


api = Flask("api")
admin = Flask("admin")
app = DispatcherMiddleware(api, {"/admin": admin})

Actual Behavior

The app object fails the following typecheck:

if isinstance(app, Flask):

127.0.0.1 - - [09/Jun/2019 15:16:30] "GET /admin/ HTTP/1.1" 500 -
Traceback (most recent call last):
  File "/Users/nshebanov/.pyenv/versions/3.7.2/lib/python3.7/site-packages/flask/_compat.py", line 36, in reraise
    raise value
  File "/Users/nshebanov/.pyenv/versions/3.7.2/lib/python3.7/site-packages/flask/cli.py", line 199, in find_app_by_string
    module=module.__name__, app_name=app_name
flask.cli.NoAppException: A valid Flask application was not obtained from "main:app".

Environment

  • Python version: 3.7.2
  • Flask version: 1.0.3
  • Werkzeug version: 0.15.4

Suggestion

Drop the check when FLASK_APP is specified and it is not a factory, or change the check to allow for middlewares (it's hard to imagine such a check though).

Checking for type Flask makes sense within find_best_app and at the moment of call_factory result evaluation within find_app_by_string.

killthekitten added a commit to killthekitten/flask that referenced this issue Jun 9, 2019
@killthekitten
Copy link
Author

I implemented the patch according to the suggestion, and it seems to work with the existing tests, however, I didn't add any new – there is a multiapp example already that isn't covered with tests, so would be great to understand why it's left out, and whether it can/should be extended for this case.

@miguelgrinberg
Copy link
Contributor

The WSGI callable for a Flask application is app.wsgi_app. Try this:

app = Flask("api")
admin = Flask("admin")
app.wsgi_app = DispatcherMiddleware(app.wsgi_app, {"/admin": admin})

See the docs.

@killthekitten
Copy link
Author

killthekitten commented Jun 9, 2019

I could do that too but it feels... a hack? It obviously doesn't do any harm, but still.

Also, the current behavior makes the application dispatching example from docs technically broken.

@miguelgrinberg
Copy link
Contributor

@killthekitten the solution I pointed you at is the recommended way to do this. The wsgi_app attribute has been in Flask for ages and it was created explicitly to prevent the Flask app instance from being buried under a potentially long chain of middlewares.

The solution that you are proposing breaks the CLI, by the way. If your api application has any custom CLI commands, those are gone once you add the dispatcher middleware. Do it in the recommended way with wsgi_app and the commands remain accessible.

@RonnyPfannschmidt
Copy link
Contributor

the example is correct

flask.run uses Flask applications, the examples explicitly use wsgi server dispatching

the documented method to apply a wsgi middleware to the wsgi details of an application is to replace the attribute as documented (its a necessary evil as wsgi middleware itself is a "decorator/proxy style pattern")

@killthekitten
Copy link
Author

👍 thanks for the details guys. I guess I'll have to live with it.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants