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

Does werkzeug have plans to support ASGI? #1322

Open
Lynskylate opened this Issue Jun 7, 2018 · 14 comments

Comments

5 participants
@Lynskylate

Lynskylate commented Jun 7, 2018

Werkzeug offer many useful method, It will be much easier than starting from scratch

@davidism

This comment has been minimized.

Member

davidism commented Jun 7, 2018

Yes, Werkzeug and Flask will eventually support ASGI. I do not have a timeline for this, although I would be happy to help review a PR if someone started one.

@edk0

This comment has been minimized.

Member

edk0 commented Jun 18, 2018

I'm interested in working on this.

I have some working but hacky ASGI support going on: werkzeug, flask. I'd like to understand more about any plans you may have before going any further.

Things that occur to me already, beyond the fact that everything I've done there could be generally nicer:

  • I'm supporting the form parser by just running its synchronous code in a thread (which I block while I read the data asynchronously). I don't think this will do. My preferred approach would be to factor out the IO.
  • Context-local support is pretty fragile. There seems to be a right way to do this, but it involves interfering with global state, and especially with ASGI we can't assume we own the world.
  • There's no obvious way, when running under ASGI, to parse form data on-demand for a synchronous function. It might be worth considering eagerly parsing form data unless we can tell the view function is async. Whatever the default is, there could obviously be a decorator to override it.

Please let me know your thoughts.

@tomchristie

This comment has been minimized.

tomchristie commented Jul 2, 2018

I'm supporting the form parser by just running its synchronous code in a thread (which I block while I read the data asynchronously). I don't think this will do. My preferred approach would be to factor out the IO.

I guess the light touch approach will be just to reimplement the existing parser, but with async IO. It'd be cleaner if the two parser classes could share a common sans-IO implementation under the hood, but it might be more practical just to duplicate & slightly modify the existing implementation.

Context-local support is pretty fragile.

Context locals (for asyncio) exist in the 3.7 stdlib. https://docs.python.org/3.7/library/contextvars.html
I'd guess use those, with an optional compat library to support earlier versions of python. (Or else just assume that ASGI on Flask would end up being a 3.7+ thing)

Does werkzeug use thread-locals? (I'm aware that Flask does)

There's no obvious way, when running under ASGI, to parse form data on-demand for a synchronous function

I'd suggest not offering a synchronous interface for parsing form data. (Or for accessing the request body in any way), and instead just offer async APIs onto it. See Starlette's API for some examples here... https://github.com/encode/starlette#body

@ThiefMaster

This comment has been minimized.

Member

ThiefMaster commented Jul 2, 2018

Since Python 3.7 is out I wouldn't be opposed to having async features requiring Python 3.7 as long as no other code is affected by it. But if there's a backport that's as good as the native 3.7 solution - even better!

Regarding async form data parsing... I guess something like await request.parse() in an async function would be sufficient, and then raise an exception when trying to access unparsed form data?

@edk0

This comment has been minimized.

Member

edk0 commented Jul 2, 2018

Sure, or just make values and friends asynchronous themselves: values = await request.values or values = await request.values(), depending mostly on which looks nicer.

@ThiefMaster

This comment has been minimized.

Member

ThiefMaster commented Jul 2, 2018

wouldn't that require rather ugly things like (await request.form)['foo'] to do an async call while getting a dict element directly without assigning in between?

@edk0

This comment has been minimized.

Member

edk0 commented Jul 2, 2018

yes :(

I'm not sure that's particularly avoidable, though. I don't think

form = await request.form
form['foo']

is really any more or less ugly than

await request.parse()
request.form['foo']

though that's obviously subject to taste.

I guess we could also invent an "async dict" whose members are all async-ified instead, but without having seen one I'd imagine it would end up rather confusing.

@tomchristie

This comment has been minimized.

tomchristie commented Jul 2, 2018

Sure, or just make values and friends asynchronous themselves: values = await request.values or values = await request.values(), depending mostly on which looks nicer.

I'd suggest using function calls for I/O-performing operations, rather than properties.

wouldn't that require rather ugly things like (await request.form)['foo'] to do an async call while getting a dict element directly without assigning in between?

Shrug - Don't do that.

asyncio is necessarily more explicit about which parts of the codebase perform I/O, so I'd tend to split those out into separate lines.

form = await request.form()
form['foo']
@davidism

This comment has been minimized.

Member

davidism commented Jul 2, 2018

While I'm all for adding async support, we still can't break Python 2 and sync versions. One approach I heard about from @njsmith is to write everything as async, then use a tool similar to 2to3 to generate the sync version. Apparently it's being tried in urllib3, but I don't know enough about it.

@ThiefMaster

This comment has been minimized.

Member

ThiefMaster commented Jul 2, 2018

I wonder if we could add magic to the objects behind request.form etc. so calling them will do async stuff while the usual dict-like methods will be sync (and would fail in async mode). Or we could just fail any access to request.form etc. in async mode and use a separate name for the async versions, e.g. request.parse_form().

Or... request_class = AsyncRequest if someone wants async; this could actually be a default in an AsyncFlask class.

@tomchristie

This comment has been minimized.

tomchristie commented Jul 2, 2018

Or... request_class = AsyncRequest if someone wants async; this could actually be a default in an AsyncFlask class.

That's the sort of approach that'd make sense to me, yeah.

@edk0

This comment has been minimized.

Member

edk0 commented Jul 4, 2018

Regarding the form parser, I've made an attempt at sansio-ing it in #1330.

@tomchristie

This comment has been minimized.

tomchristie commented Oct 15, 2018

There's also a streaming form parser implementation at https://github.com/andrew-d/python-multipart with 100% coverage. (I found one other but it wasn't obvious that it could be easily adapted into a "feed data, handle events" flow.)

python-multipart is the library I'm now using for Starlette. You can take a look at the integration with the async stream here: https://github.com/encode/starlette/blob/master/starlette/formparsers.py#L207

@tomchristie

This comment has been minimized.

tomchristie commented Oct 15, 2018

I've also been thinking about best ways to present a sync and async compatible interface, since I also want that for Starlette (although going in the other direction to Werkzeug, for me it's about "I have an existing async interface, how do I now also present a sync one")

For Starlette I think I'll probably push the actual parsing into an async parse(self) method, and typically call that sometime during request dispatch, but expose regular plain properties form, files etc... for accessing the results from user code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment