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

Should quart.make_response be typed as returning Response? #288

Open
pquentin opened this issue Nov 17, 2023 · 2 comments
Open

Should quart.make_response be typed as returning Response? #288

pquentin opened this issue Nov 17, 2023 · 2 comments

Comments

@pquentin
Copy link

pquentin commented Nov 17, 2023

Steps to reproduce

With the following Quart app:

from quart import Response, make_response
from quart_trio import QuartTrio

app = QuartTrio(__name__)


@app.route("/")
async def hello_world() -> Response:
    return await make_response("<p>Hello, World!</p>")


if __name__ == "__main__":
    app.run()

mypy fails because make_response does not return a Response:

quart_app.py:9: error: Incompatible return value type (got "quart.wrappers.response.Response | werkzeug.wrappers.response.Response", expected "quart.wrappers.response.Response")  [return-value]

Expected result

I would expect validation to pass. Indeed, the equivalent Flask app passes validation:

from flask import Flask, Response, make_response

app = Flask(__name__)


@app.route("/")
def hello_world() -> Response:
    return make_response("<p>Hello, World!</p>")

Workarounds

  1. Use return "<p>Hello, World!</p>" and mark the return type as str. But figuring out the right type is tiring, is a maintenance burden and arguably introduces unnecessary variation in the code.
  2. Use return quart.Response("<p>Hello, World!</p>") but it quickly breaks for more complex types. I spent a lot of time trying to understand why quart.Response({"headers": "foo", "params': "bar"}) was returning "headersparams"!
  3. Type the return as quart.typing. ResponseTypes. This is what I'm doing right now in the urllib3 tests (Migrate TestPoolManager to Hypercorn urllib3/urllib3#3193) but I would rather use Response.

Environment

  • Python version: 3.12.0
  • Quart version: 0.19.3 (quart-trio 0.11.0)
@pgjones
Copy link
Member

pgjones commented Apr 1, 2024

I think the route should be typed as returning a ResponseReturnValue, does that work for you?

@pquentin
Copy link
Author

pquentin commented Apr 4, 2024

Yes, that works! However that's a lot to type. And it does not apply to after_request where Response is needed:

def apply_caching(response: Response) -> ResponseReturnValue:
    ...

But it works, so I'm happy to close this if you want to.

pquentin added a commit to pquentin/urllib3 that referenced this issue Apr 4, 2024
pquentin added a commit to urllib3/urllib3 that referenced this issue Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants