Eyepea / aiosip

SIP support for AsyncIO (DEPRECATED)
Apache License 2.0
83 stars 43 forks source link

Akwardness with the new Dialplan #117

Open vodik opened 6 years ago

vodik commented 6 years ago

Don't take this wrong, @ovv - I think its actually a big step the right direction.

But I've built now, what is essentially a light weight PBX ontop of aiosip, and while a lot of abstractions fit nicely into how the library is designed currenty (most of my issues are actually covered in #115), and I think Application and Dialplan might be backwards.

When I build a registrar, I found it easiest to build it into the Dialplan, and just make the Dialplan aware of the underlying aiosip.Application - but this can only be done awkwardly. I'm currently using something like this:

@attr.s(auto_attribs=True)
class Application:  # noqa: D101
    _app: aiosip.Application = attr.ib(init=False)

    @_app.default
    def _get_app(self):
        return aiosip.Application(
            dialplan=Registrar(self),
            debug=True,
        )

    @property
    def dialplan(self):
        return self._app.dialplan

    async def connect(self, *args, **kwargs):  # noqa: D102
        return await self._app.connect(*args, **kwargs)

    async def run(self, *args, **kwargs):  # noqa: D102
        return await self._app.run(*args, **kwargs)

The awkwardness comes from the fact that I can't pass the aiosip instance into the dialplan as it hasn't been created yet. I instead pass the self through, and pretend that its the aiosip.Application.

But the only method that the Registrar class uses is connect (run is used externally).1

This got me thinking that maybe the dialplan is actually closer to what should be considered "the application" and the connect functionality should be pulled out of the application - to borrow aiohttp terminology, maybe Dialplan is the real Application and what we currently use as an Application/connect really should be modelled as an AppRunner and TCPSite (though something like UserAgent or Profile would be more appropriate names for SIP).

Nothing technically says everything has to live inside a single application - but I'm not sure if its viable either yet. This is just me thinking out-loud. I'm curious to see how you guys modelled things, if you have any concerns.

1I could, for my use case, ignore the connect call and just use the peer associated with an inbound registration's dialog. But a registration doesn't have to come from the address specified in the Contact header - its totally valid to be able to register on the behalf of another device.

ovv commented 6 years ago

Don't take this wrong, @ovv - I think its actually a big step the right direction.

No problem, it's by iteration that we will build something good.

I'm curious to see how you guys modelled things, if you have any concerns.

We use an API-Hour container so indeed I kind of pass the Application to the Dialplan but I don't actually use it. We are not that much concern about being completely SIP compliant for the moment so we use the associated peer.

...

Your proposition look interesting but it add a lot of complexity. The Request object has an app attribute (like aiohttp) so it is available in the Dialplan for an incoming request. Would using that be possible in your setup ?

One thing that could be useful when using the Application standalone would be on_startup / on_shutdown signals. It might be interesting to look into aiohttp code to model our applications in the same way.