On ASGI's double-callable interface. about asgiref HOT 38 CLOSED

Okay, had "Da-da-DAAAA" moment this morning.

Class instances can be awaitable.

class App:
    def __init__(self, scope, receive, send):
        self.scope = scope
        self.receive = receive
        self.send = send

    def __await__(self):
        return self.dispatch().__await__()

    async def dispatch(self):
        ...

This gives you: await App(scope, receive, send), which will instantiate the class, and then run dispatch(). So the single-callable style can still use an "instantiate then run" pattern, and can support uninstantiated classes as applications.

One nice property about the __init__ + __call__ pattern is that Flask can tell whether it's in WSGI or ASGI mode automatically

Sure, so:

class App:
    def __init__(self, **config):
        pass

    def wsgi(self, environ, start_response):
        print({"environ": environ, "start_response": start_response})

    async def asgi(self, scope, receive, send):
        print({"scope": scope, "receive": receive, "send": send})

    def __call__(self, *args):
        assert len(args) in (2, 3)
        if len(args) == 2:
            return self.wsgi(*args)
        return self.asgi(*args)


async def run():
    app = App()
    print("WSGI:")
    app("environ", "start_response")
    print("ASGI:")
    await app("scope", "receive", "send")


loop = asyncio.get_event_loop()
loop.run_until_complete(run())

from asgiref.

Summary of what I now think would be sufficient for a 3.0 release:

• Single callable async def app(scope, receive, send), with backwards support for the double callable style.
• Supplement the existing lifespan.startup.complete and lifespan.shutdown.complete messages with complementary lifespan.startup.failed and lifespan.shutdown.failed {"type": ..., "exc": ...} messages.

That'd enough that we don't neccessarily need to include any handshaking or supported-protocol info beyond the currently existing spec.

from asgiref.

Closing since I'm not expecting actionable responses on asgiref as a result of this, but certainly open to any discussion here.

from asgiref.

/cc @pgjones

from asgiref.

This is going to be one of the tricky things with making Flask compatible with ASGI. Init is for creating the app, not starting the scope, so we'd need to figure out some way to make call do double duty to push the scope then handle the request.

from asgiref.

Thanks @davidism, that's helpful feedback.

Init is for creating the app, not starting the scope

Same in Starlette, yeah.

It's not massively problematic as it stands, it's just less direct:

ASGI:

class Flask:
    def __init__(self, **config):
        ...

    def __call__(self, scope):
        return functools.partial(self.asgi, scope=scope)

    async def asgi(self, receive, send, scope):
        ...

Single callable:

class Flask:
    def __init__(self, **config):
        ...

    async def __call__(self, scope, receive, send):
        ...

(Obviously there's a whole different set of things to consider wrt. how best to keep both ASGI and WSGI compatability, but that's a different question altogether.)

from asgiref.

I forget my exact motivation for having the double-call method these days, though I do remember I seriously weighed both before picking the current layout.

I think in the end it was down to wanting to have an "instance" to pass around the server that I could then use as a key and assign event loops and attributes to, which is a bad case of letting implementation define specification. I don't think there's inherently anything totally necessary about having double-call as a mechanic, though I do agree now is maybe the last chance to change it.

Let's talk about feasibility here - if it were to change, we still need to maintain some sense of backwards compatibility for servers (frameworks/apps I think are a bit more free to just say "use the newest server versions"). Without attributes on the application object, or use of inspect, is there any reasonable way to know an app is one type over another?

Also, we could totally ship an adapter from single- to double-call in asgiref, like this:

def single_to_double(app):
    class double:
        def __init__(self, scope):
            self.scope = scope
        def __call__(self, receive, send):
            app(scope, receive, send)
    return double

from asgiref.

Also, we could totally ship an adapter from single- to double-call in asgiref.

I'd not really considered that aspect of it - yeah that's a good point.

Presumably we'd be able to use asyncio.iscoroutinefunction(func) to differentiate between the two cases reliably.

from asgiref.

I see this as similar to React's functional and class components, which basically consist of either

async function App(props) {
  return ...;
}

// or

class App extends React.Component {
  constructor(props) {
    ...
  }

  async render() {
    const state = this.state;
    return ...
  }
}

depending on whether you have state or not to worry about. I think then ASGI could follow suite and allow,

async def app(scope, receive, send):
    ...

# or

class App:
    def __init__(self, scope): ...

    async def __call__(self, receive, send): ...

The key then is to know (server side) which invocation to use. React does this in a quite clever way which boils down too,

// Inside React
class Component {}
Component.prototype.isReactComponent = {};

// We can check it like this
class Greeting extends Component {}
console.log(Greeting.prototype.isReactComponent); // ✅ Yes

The simplest thing is probably to just insist that ASGI class interfaces do something similar,

class App:
    is_asgi_class = True

    def __init__(self, scope): ...

    async def __call__(self, receive, send): ...

# or

class Special:
    # Note no is_asgi_class
    async def __call__(self, scope, receive, send):
        ...

So I think it is worthwhile to change the specification from double callable to class or functional applications. I think this as the functional form is quite clear to use and we can document it as React does (state or not). I've also not seen much use of a double functional callable in practice, so removing it (although you could attach is_asgi_class to the function object) should be ok.

I can't think of a reliable way to make this backwards compatible though.

from asgiref.

i feel we could use asyncio.iscoroutinefunction to provide backwards compatibility here - if it's not a coroutine function, then do a double-callable, and if it is, do a single-callable with (scope, receive, send). We could also allow override attributes as @pgjones suggests to force the other way.

from asgiref.

I don't think asyncio.iscoroutinefunction would identify the Special case above though.

from asgiref.

Just checked and you're right, but we could also check that and the object's __call__ directly, as that works:

>>> asyncio.iscoroutinefunction(Special())         
False                                              
>>> asyncio.iscoroutinefunction(Special().__call__)
True                                               

It would need a bit more intelligence about is-this-a-class than that, but it would work.

from asgiref.

I went down this route and couldn't convince myself there was a reliable way; considering the App above Special, asyncio.iscoroutinefunction(App().__call__) is True yet it should be called differently.

from asgiref.

Well remember, you'll get passed the class for double-call apps like App and an instance for something single-call like Special (as it would not be valid otherwise as it's not async callable). So I figure:

• If it's an uninstantiated class, call in double-call mode (as class constructors cannot be async)
• If it's an instantiated class, test to see if __call__ is a coroutine function and decide based on that
• If it's not a class at all, test to see if it's a coroutine function and decide based on that

This doesn't have to be totally perfect, just capture about 95% of use cases and sensibly explode in the other 5% of cases so people can implement a quick fix for their apps.

from asgiref.

This seems to work, see this. It seems like it there should be a simpler way though.

from asgiref.

Not had a chance to test it, but maybe this?...

asyncio.iscoroutinefunction(getattr(app, “__call__”, app))

from asgiref.

Oh okay, probably not. Eg. Checks the wrong thing in the case of an uninstatiated class.

from asgiref.

Right, but I believe we can still check if it's an instantiated class or not and then do a check.

So, given that this appears to be feasible to do in a backwards-compatible manner, I want to ask if we think it's sensible to make the shift. Specifically, should we:

a) Allow for both kinds of application (single- and double-call) natively
b) Only allow the existing (double-call) apps but ship a single-to-double adapter in asgiref
c) Move to only allowing single-call natively and use the backwards-compat solution above to allow for a transitionary period for servers

from asgiref.

Well, I think it's either (d) "leave things just as they are" or (c) "Move to single-call, with a compat period".

Of those (c) would very likely be my prefered option: I think the knock-on effects of having a simpler call-flow would be a big deal if ASGI does end up with very large-scale adoption.

I would probably implement the single-callable in a Starlette branch as a demonstration first, before taking a definitive stance.

Minorish aside: We've currently have something of a hack for servers to determine if the lifespan protocol is supported by an application or not, in that servers differentiate between an exception when the application is instantiated vs. an exception when the ASGI callable is invoked. That wouldn't make sense any more with a single callable. (It doesn't work reliably anyways, eg. some ASGI middleware will have good implementation reasons to only instantiate the child app within the ASGI callable, which masks which of the two cases the child app has raised an exception within.)

from asgiref.

One nice property about the __init__ + __call__ pattern is that Flask can tell whether it's in WSGI or ASGI mode automatically: if __call__ gets two args, it's WSGI, otherwise it's ASGI and should (for now) produce an intermediate scope callable. Is this a good thing to rely on? Will it even be useful when we do implement ASGI? 🤷‍♂️ It's probably better to set that in __init__ or os.environ.

from asgiref.

I think in the end it was down to wanting to have an "instance" to pass around the server that I could then use as a key and assign event loops and attributes to, which is a bad case of letting implementation define specification.

Flask already has the concept of the request context, an object that represents the request and some extra state, which gets put on a thread-local stack. My intuition would be to use the same thing in ASGI, upgrading it to be aware of Python 3.7 context locals. So the double callable would just add a level of indirection with the partial wrap.

from asgiref.

Okay, didn't want this to have to be treated as an open ticket unless it was actually considered a viable option. I figure since it's actually under consideration I might as well open it up.

from asgiref.

Totally. I think this is feasible, the question I really have is how to roll it out. Do we just put the works-both-ways code into current ASGI servers and say it's done? The spec probably needs a major version bump, too?

from asgiref.

Do we just put the works-both-ways code into current ASGI servers and say it's done?

Yes, I think so.

The spec probably needs a major version bump, too?

Yes.

Right now the server can advertise what version of the ASGI protocol it implements, and can indicate which extensions it supports. However we've got no way for the client to advertise which version it supports, and which message types it accepts. That's a problem eg. with 2->3, because servers that send lifespan messages to clients that do not accept them have no way of determining if any raised exception was due to application startup failing, or due to lifespan being unsupported.

A remedy for this, which would also help make ASGI more robust to any other incremental changes would be for the very first thing that the client does if it accepts lifespan messages, would be to send a handshake, eg. {"type": "lifespan.handshake", "asgi": {"version": 3}, "protocols": ["http", "websocket", "lifespan"]}.

Simple client implementations can just do:

async def asgi(scope, receive, send):
    assert scope["type"] == "http"
    ...

Which would be enough for a server to determine that "no I didn't get a handshake, I got an exception. I'll turn lifespan off", rather than "I started the lifespan, sent 'startup', and got an exception, I don't know if that's because the application doesn't support lifespan, or it does but failed to complete application startup".

Summary:

• Yeah, a version bump, and servers that support both styles would make sense.
• An ASGI handshake message on the lifespan protocol would help deal with an existing issue of "can't really determine which message types the client supports', that gets more problematic in the v3 style. (Since there's no "raise an exception on init", before the main call is started anymore)

Making those two changes together looks like it'd make sense to me.

from asgiref.

OK, I think we agree on the version bump then (to 3). I can pop back in to Daphne and get that patch written once the spec is updated, but I want to solve the other thing first.

My design assumption with things like lifespan was that the app would just ignore protocols it didn't support, rather than raising exceptions. I'm not sure if I want to make the app communicate back to the server when it starts, since that starts to defeat the whole keep-it-relatively-simple aspect.

from asgiref.

My design assumption with things like lifespan was that the app would just ignore protocols it didn't support, rather than raising exceptions. I'm not sure if I want to make the app communicate back to the server when it starts, since that starts to defeat the whole keep-it-relatively-simple aspect.

Indeed yeah.

Question being how (or if) we best communicate which protocols (and version) the client supports.

E.g. if we just ignored lifespan messages, then servers that supported it would (by default) send a startup, and just hang waiting or timeout with a "failed to startup message".

It could be that the onus is on developers to enable to disable the server protocols.

Tho it's also a bit awkward to have to tell devs that the right way to run a basic "hello, world" ASGI app with, say, uvicorn, is actually:

$ uvicorn example:App --lifespan none --ws none

I guess if we really want a robust ability to upgrade the protocol over a long period of time then we really will want a both-way server & client exchange of capabilities, but I also agree with the general keep-it-relatively-simple motivation.

from asgiref.

Question being how (or if) we best communicate which protocols (and version) the client supports.

Can't we simply still use exceptions for this? From what I understand the major problem is that it's hard to discern an "I can't handle this protocol" from "whoops, I failed trying". What about specifying a specific exception type? NotImplementedError seems to fit nicely :) I guess applications that are more than a toy example should make sure to catch NotImplementedError to avoid it from falling through. Or, we can require the exception to have a certain attribute set.

from asgiref.

Can't we simply still use exceptions for this?

Yeah, we can. The only problem is lifespan - if the server starts the callable and sends a "started", then once it handles the exception there's no way to differentiate between "startup failed" - in which case the server should just stop. (Eg. couldn't connect to the database). and "lifespan not supported".

If the first thing that the lifespan protocol does is send back a "hi there" message, then you can differentiate between the two cases.

The NotImplementedError idea might be okay. Perhaps there's a risk that if your codebase erronously raises it elsewhere then it could end up silently turn off lifespan or websocket handling.

from asgiref.

Perhaps there's a risk that if your codebase erronously raises it elsewhere then it could end up silently turn off lifespan or websocket handling.

Maybe something like this?

In the application:

    ...
    else:
        raise NotImplementedError("asgi-" + scope["type"])

In the server:

try:
    ...
except NotImplementedError as err:
      if err.args and err.args[0] == "asgi-lifespan":
             ...  turn lifespan off
       else:
            raise

from asgiref.

Yes, lifespan really does gum up the works because it's the only protocol the server actually waits on; other protocols aren't going to have the issue where if you don't support them your entire app freezes.

I'm not super keen on re-using NotImplementedError to mean "ignore this", but I don't see many ways out of this without annotating applications saying "hey I support lifespan!". I wouldn't be against that either, but I'd want the default to be no-lifespan and then you have to explicitly turn it on.

from asgiref.

Yes, lifespan really does gum up the works because it's the only protocol the server actually waits on; other protocols aren't going to have the issue where if you don't support them your entire app freezes.

Yeah. I'm reasonably happy with the "send a 👍 as the very first thing the lifespan protocol does" because it's at least limited to lifespan. It doesn't even necessarily need include any handshake info such as a list of supported protocols, it could just be a "I'm ready to startup when requested". It solves the issue of "did lifespan raise an exception because startup failed, or just because it wasn't supported?".

The rule of thumb for devs to follow wrt. supporting protocols can simply be:

• Applications/Endpoints should strictly check the protocol is a supported type, and error otherwise.
• Middleware should pass unknown protocol types through unmodified.

So, eg, this would be enough to gracefully disable lifespan and websockets:

async def hello_world(scope, receive, send):
    assert scope['type'] == 'http'

from asgiref.

An alternative: Have an explicit “startup failed” message for the lifespan protocol, which indicates to the server that it should abort. That’s then properly distinct from the lazy exception case.

from asgiref.

We don't strictly need the shutdown failed, but it'd seem to make sense for completeness.

from asgiref.

How does "startup failed" help differentiate the non-lifespan-enabled app and the lifespan-enabled app that wishes to exit, though? A non-lifespan enabled app will never send complete or failed and so the server will sit there forever.

from asgiref.

It's sufficient so long as apps check they're getting supported protocols, and raise errors (of any kind) if they don't.

from asgiref.

Ah, so we say that "if you get an error back from startup you continue" whereas "if you get a failed back from startup you stop"? I can go with that.

from asgiref.

I have made these changes, including a backwards-compatability module in asgiref, in this pull request: #80

Comments more than welcome!

from asgiref.

Marking as resolved, as ASGI 3 is now out.

from asgiref.