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

Autobahn shared core API #472

Closed
oberstet opened this issue Aug 30, 2015 · 59 comments
Closed

Autobahn shared core API #472

oberstet opened this issue Aug 30, 2015 · 59 comments

Comments

@oberstet
Copy link
Contributor

As we have been discussing various improvements to the AutobahnPython WAMP API like the new Connection class and just using a WAMP Session object instead of being forced to inherit from a Session base class, now is the chance to design a core API for WAMP that is highly similar between:

  • AutobahnPython
  • AutobahnJS
  • AutobahnC
@oberstet
Copy link
Contributor Author

@oberstet
Copy link
Contributor Author

oberstet commented Sep 1, 2015

Here is my current favorite sketch:

from twisted.internet import reactor
from autobahn.twisted.wamp import Connection

connection = Connection('ws://localhost:8080/ws', u'realm1')

@inlineCallbacks
def on_open(session):
    print('session connected: {}'.format(session))
    connection.close()

connection.on('open', on_open)

connection.open()

reactor.run()

This would establish a WAMP-over-WebSocket connection and attaching a session to a realm. When the session has joined, it prints and then closes the connection.

Above would not allow the session to leave and join the same or another realm while the underlying connection stays. As the on_open event on Connection would only fire when the session has joined the realm.

@oberstet
Copy link
Contributor Author

oberstet commented Sep 1, 2015

Here is a bigger example with multiple transports, automatic reconnection and authentication:

from twisted.internet import reactor
from twisted.internet.defer import inlineCallbacks

from autobahn.wamp.auth import compute_wcs
from autobahn.twisted.wamp import Connection

# list of transports to try
transports = [
    {
        'type': 'rawsocket',
        'endpoint': {
            'type': 'unix',
            'path': '/tmp/cb-raw',
        }
    },
    {
        'type': 'websocket',
        'endpoint': {
            'type': 'tcp',
            'host': 'localhost',
            'port': 8080
        },
        'url': 'ws://localhost:8080/ws'
    }
]

# list of credentials to use for authentication
credentials = [{
    'type': 'wampcra',
    'id': 'user1',
    'on_challenge': lambda challenge: return compute_wcs('password1', challenge)
}]

# options that control the connection's behavior
options = {
    # http://autobahn.ws/js/reference.html#connection-options
    'max_retries': 5
}

# create a connection object with all the bells we defined before
connection = Connection(transports, u'realm1', credentials=credentials,
    extra=None, options=options, reactor=reactor)

# when the connection has established a transport, created an ISession object
# and the latter has joined a realm, this will fire
@inlineCallbacks
def on_open(session):
    print('session connected: {}'.format(session))

    # app code now just uses the session object for WAMP
    try:
        res = yield session.call("com.example.add2", 2, 3)
        print('got result: {}'.format(result))
    except Exception as e:
        print('something bad: {}'.format(e))

    connection.close()

# when the connection closes or goes through a reconnect cycle, this event fires
def on_close(reason):
    print('session closed: {}'.format(reason))

# attach our event handlers to the connection object
connection.on('open', on_open)
connection.on('close', on_close)

# now actually open the connection
connection.open()

# enter event loop
reactor.run()

@meejah
Copy link
Contributor

meejah commented Sep 2, 2015

Another sketch, with the listener-API on the ApplicationSession instances itself:

from twisted.internet.task import react
from twisted.internet.defer import inlineCallbacks, Deferred
from twisted.internet.endpoints import clientFromString


def one_off_function(*args, **kw):
    print("one_off_function", args, kw)

@inlineCallbacks
def register_api(session):
    yield session.register(one_off_function, u"com.example.api.v0.one_off_function")
    # ...                                                                                                           

@inlineCallbacks
def register_management_api(session):
    yield session.register(lambda: None, u"com.example.private.do_nothing")
    # ...                                                                                                           

def main(reactor):
    session = ApplicationSession()
    # or "session = MyComponent()" if you prefer subclassing to composition                                         
    connection = Connection(session, [
        {
            "type": "rawsocket",
            "endpoint": clientFromString("tor:timaq4ygg2iegci7.onion:9443"),
        }
    ])

    session.on.open(register_api)  # or session.on('open', register_api)                                            
    if True:
        session.on.open(register_management_api)
    session.on.ready(lambda session: session.publish(u"com.example.api.v0.ready"))
    reactor.callLater(10, connection.close)
    # .open will errback if connect (or re-connect) fails; otherwise callback w/ None on clean disconnect
    return connection.open()

if __name__ == '__main__':
    react(main)

@oberstet
Copy link
Contributor Author

oberstet commented Sep 2, 2015

Regarding what events we want to expose (independent of where we expose those events) - I dug out our Slack conversation @meejah

so the flow would be: "connect" when the transport is connected, "join" when we've authenticated (but not yet run onJoin), "ready" when onJoin has run and completed any async-stuff. then "leave" when the session closes, and "disconnect" when the transport goes away

@meejah
Copy link
Contributor

meejah commented Sep 2, 2015

Further to @oberstet's last comment above, "any async stuff" includes async-returns from previous handlers. For example, if there are 3 'join' listeners that return Deferreds, "ready" shouldn't fire until those 3 Deferreds are done.

@oberstet
Copy link
Contributor Author

oberstet commented Sep 2, 2015

    session = ApplicationSession()
    # or "session = MyComponent()" if you prefer subclassing to composition    

Have been looking at these 2 lines for minutes .. and I know what slightly irritates me:

The word "session" - for me - implies some ephemeral thing that is created for me, and which I then use. In above I create the session, and it's not ephemeral. It exists without any transport existing yet, and without having joined a realm.

The funny thing is your commented line: session = MyComponent()

Note the mix of "session" and "component": #383

In my perception, the word "component" is much more in line with the user creating the thing ("component like") rather than it being created for the user ("session like").

@oberstet
Copy link
Contributor Author

oberstet commented Sep 2, 2015

Another aspect. At the WAMP protocol level, there are indeed "session" and "transport".

A session rides on top of a transport. And a transport can outlive a session: session 1 ends, and without closing the underlying transport, a new session 2 can start. The new session will get a new WAMP session ID though!

However, when a transport goes away, the session is gone too. A WAMP session (today) cannot outlive a transport (but: crossbario/crossbar#300).

If we had a user precreate

session = ApplicationSession()

the lifetime of that Python object hasn't a direct correspondence with "session" in the WAMP protocol sense. I think this discrepancy might be confusing.

@meejah
Copy link
Contributor

meejah commented Sep 2, 2015

Component vs Session: yeah, I can see the confusion; was trying to copy the examples usage. Perhaps there's no value in even "letting" people choose between composition and inheritance? (As in: no inheritance allowed in new API?). I think there is value in leaving that ability in (even if the methods are re-named to PEP8 like on_join vs. onJoin etc) since presumably some people will still enjoy that style...?

Anyway, for the composition-only case, the session could just be created + owned by Connection and you'd either do connection.session.on('join', ...) and/or have Connection provide pass-through calls for convenience).

There's also an interesting edge-case to consider: what if an event has already happened, and a new listener is subscribed? Should it get immediately invoked (e.g. like when you .addCallbacks to an already-completed Deferred)? This is probably most-relevant for join or ready. Depending on documentation admonishing people to only add join listeners before .open() is called might not work ;)

@meejah
Copy link
Contributor

meejah commented Sep 2, 2015

Lifecycle-wise: in some ways it does seem "more pure" to have the ApplicationSession lifetime match that of the logical WAMP session (i.e. if you re-connect a session on the same transport, you get a new ApplicationSession instance) -- if you're talking about getting rid of the existing cycle in the WAMP state-machine (e.g. that onJoin can be called > 1 time) that overall might be much cleaner. However, then you'd likely want/need a "session_created"-type event from Connection? (i.e. distinct from, and strictly always occurring before the onJoin)

Also, if there is a Connection.session then does user-code have to guard against .session being None (or, "no WAMP session established just now") in such a scenario?

All that said, the existing WAMP state-machine does have a "not [yet] connected" state and so from that perspective having a single WAMP "session object" that can join/leave and then join again isn't so outlandish...

@oberstet
Copy link
Contributor Author

oberstet commented Sep 6, 2015

@meejah one thing we need to keep in mind: we need an approach which allows users to take unmodified code to run hosted under CB.

That means, the user code needs to be wrapped inside a class (as today with ApplicationSession) or ..?

@oberstet oberstet removed this from the 0.11.0 milestone Sep 6, 2015
@meejah
Copy link
Contributor

meejah commented Sep 6, 2015

Hmm. Yeah to host under CB right now with 'class' things you'd need an ApplicationSession subclass I guess.

You could run the other ("listener"?)-style code as a guest worker right now.

I suppose the equivalent we'd need in CB is a "function" (router + container) worker which imports a particular function that takes a session (and then adds the listeners it needs etc). So you'd have to write your code with that in mind I guess ...

@oberstet
Copy link
Contributor Author

oberstet commented Sep 6, 2015

Whatever we come up with recommended 80% API in AB must work without changing any code when the component is taken to run under CB. If that can only be achieved via subclassing (the current ApplicationSession approach), then we'll stick to that.

@meejah
Copy link
Contributor

meejah commented Sep 6, 2015

Yeah, that's a good point; have to think how best that fits in with a "composition"-style thing.

Another (but maybe more complicated-sounding) option would be that CB provides configuration so you can specify the listeners directly, something like:

{
    "type": "function",
    "on_join": ["package.register_api", "package.register_management_api"],
    "on_leave": ["package.sub.on_leave"],
}

@oberstet
Copy link
Contributor Author

oberstet commented Sep 6, 2015

That last idea points to bigger issues with a non-inheritance based approach: having functions like above in a module means they are forced to hold state in a module global var (or misuse the provided generic WAMP session/connection object to attach app state).

Sidenote: me, coming from C++, this is a total no go. In Py, it seems an accepted approach (Django, Flask, ...). It's kind of replicating the mistakes of GIL and the failure to correctly wrap all interpreter state (like all sane, embeddable VMs do .. Lua, V8, ..) in a non-global at the app level. I don't want to encourage that.

Now, the problem with having state in globals in above situation: what I run multiple instances of my component in a single worker (router/container)? Doesn't work.

Then: having it at that level of detail in the CB config means: essentially mixing CB node config with app internals. A CB admin would not and should know about this in detail.

@meejah
Copy link
Contributor

meejah commented Sep 6, 2015

Yeah, you would have to attach any state to the provided session instance, IMO.

Putting any kind of state -- even "singleton"-pattern stuff -- in a module isn't very good (even if it is used occasionally).

However, this isn't very different from the "override" case, where essentially you have to initialize all your state in onJoin or the class will fail to work properly if/when you re-join.

So the recommendation I think doesn't change: you initialize any state you need into the session object, and you should do this in onJoin (or your join listener).

@meejah
Copy link
Contributor

meejah commented Sep 6, 2015

...but yes in a "non-dynamic" language (C++) it's not immediately obvious what a good solution is for things needing state (aka "instance variables") since they'd need to be declared (i.e. sub-classing).

@oberstet
Copy link
Contributor Author

oberstet commented Sep 6, 2015

slightly related: CB deliberately does NOT provide the ApplicationSession instance a variable that would be shared across multiple components. Because then, you couldn't freely move around components deployment wise. One should be able to run 2 components in 1 container worker or 2 container workers without changing a single line. And this works today, since any inter-component communication MUST be via WAMP. When the component co-reside in a single worker, those WAMP interactions boil down to mere function calls, whereas when they don't co-reside, it'll be WAMP over the wire (where wire could be a local Unix domain socket, or a wire to the other end of the world). We definitely don't want to loose that deployment transparency. It's part of the original vision of WAMP and CB. And I don't know other systems that allow that in that extreme way.

@meejah
Copy link
Contributor

meejah commented Sep 6, 2015

Maybe I'm using the wrong term, I didn't mean a "shared state" instance variable, I just meant that you can't magically create a thing in C++ via e.g. this->something_new = ... like in Python or JS so I guess a C++ implementation would need subclassing and have to at least recommend "re-initialize in onJoin". The only alternative I can image is for the C++ ApplicationSession implementation to provide a hash-map or something as this->state but that's ... really hacky.

@meejah
Copy link
Contributor

meejah commented Sep 7, 2015

Perhaps a way to reconcile "state" between dynamic (JS, Python) and not (C, C++) without resorting to subclassing is to add a "state" attribute to the ISession / ApplicationSession public API?

In C this can be a void* and you can do what you want with it (maybe C++ too, or perhaps something fancy with templates so its got a type). For Python/JS this can just be None/null by default, and you can assign whatever you want.

Encouraging this explicit encapsulation of session-state might help too with session freeze/thaw: we could say "if the 'state' object implements ISerializable [or whatever] then your session can be frozen/unfrozen".

@sametmax
Copy link
Contributor

sametmax commented Sep 8, 2015

Some notes I got in mind:

Let's be careful to keep congruent signatures

E.G, currently in Python you got:

self.register(func, 'name');

But in JS you got:

session.register('name', func);

An no error if you mix them up.

I have several painful memories because of it.

We should have sane default

When you connect to redis on Python, you can do:

import redis
connection = redis.Redis()

You don't need to do:

import redis
connection = redis.Redis(host="localhost", port=6379)

It relies on sane officially documented default.

For autobahn and crossbar, it means it we should be able to do:

connection = Connection()

And choose some default, make it very clear in the doc and stick to it.

Especially, we need to define an official default port (not 8080, which is often used by dev server) and an official default realm. "realm1" is not a really good name. "public" would be better, since all tutorials use it like some kind of chmod 777 anyway, and it's easier to remember.

It will also make OS packager's life easier. Having a default port is good for them.

Some stuff are better if not shared between languages

We should definitely have a common API. But since some constructs are idiomatic in some languages, it is important to provide them as well.

For Python, it's decorators:

@inlineCallbacks
def on_open(session):
    print('session connected: {}'.format(session))
    connection.close()

connection.on('open', on_open)

Is a nice API. But since you are already using a decorator, this will be handy:

@connection.on("open")
def on_open(session):
    print('session connected: {}'.format(session))
    connection.close()

The good news is that we don't have to choose between them, we can do both:

class Connection:
    ...

    def add_event_listener(event, callback):
        # do the real work here

    def on(event, callback=None):

        # classic form
        if callback:
            return self.add_event_listener(callback)

        # decorator form
        else:
            def decorator(func):
                cb = inlineCallbacks(func)
                return self.add_event_listener(event, cb)

            return decorator

For JS, it's promises:

connection.open().then(function(){}

)

This will make JS coders more "at home".

You still should totally have the ordinary common syntax. But have these as well.

Distinguish connection failing and closing

Currently in the JS API it's the same event, and you check a status in a variable to know what's really happening. A wrapper checking the status an trigerring explicitly named event would make things more obvious.

Open should start the event loop

In JS the event loop is implicit. To give the same feeling of easyness, connection.open() should do reactor.fun() (or the asyncio equivalent), but have a "start_event_loop" parameter with a True default value, in case somebody wants more control.

Indeed, we are aiming to facilitate the beginer use case, who will want to start as quickly and easily as possible. Advanced used are the one which should need additional work.

I know that the strength of Twisted is that you can use many different modules and plug them together in the same reactor, but this will not be the objective of people trying WAMP. They will not think "let's write a custom protocol and plug in an IRC bridge with my websocket website" on the first day. Nor on the second.

For them, the event loop we are using is an implementation detail. Exposing it should be explicitly requested with code, such as a "start_event_loop" parameter.

Prevent importing stuff you need all the time and abstract some more

If you use some things really often, they should be aliased in a namespace that is always imported and be abstracted away to hide differences between event loop.

Stuff line @inlineCallbacks, @coroutine or returnValue should have wrappers, and the wrappers should be attached either to session (or an object containing session passed instead of session) or connection.

Importing them all the time and having to know about the difference between the event loops is only useful for advanced usages. You can create decent websites and app without even needing to understand perfectly the event loop, just that you should not block and where to setup your "yield".

The idea remains :

  • hide stuff you don't use often;
  • make stuff you use often quick and obvious to do. Cheat a little if needed;
  • add mechanisme to expose the underlying full API for advanced uses.

Check if name == "main" implicitly

Libs like begins started to do this with decorator like @begin.start.

Instead of calling connection.open(), you have some method that trigger parameters parsing and command running only if the module is not imported.

class Connection:
    ...

    def cmd(params):
        # argparse stuff here checking command line params
        # stuff checking env var params
        # all that is merge with params passed as arguments
        if __name__ == "__main__":
            return self.open(merged_params)
        return merged_params

But the more you do that, the more putting this code on a Connection object seems a bit weird. That's why last year I suggested the App centered

Final API

If you do all this, the hello world becomes:

from autobahn.twisted.wamp import Connection

connection = Connection()

@connection.on('open')
def on_open(session):
    print('session connected: {}'.format(session))
    connection.close()

connection.cmd()

That's easy. That's clean. That calls for a copy / paste to try it right away !

But I'd still advice to use something similar to the app API, plus this allow to override call and make it easy to be embeded in crossbar.

@oberstet
Copy link
Contributor Author

oberstet commented Sep 9, 2015

Ok, here are a couple of ones where I do have a strong opinion (note: I will continue with all your @sametmax points tomorrow .. the other I need to think more, and now I need some sleep):

Open should start the event loop

No. We can't do that, as e.g. in Crossbar containers, the event loop is started by the native worker.

This is the reason why there is ApplicationRunner: it does the plumbing when there is no hosting app/container around.

In JS the event loop is implicit.

This is correct for browsers and NodeJS runtimes .. which granted are the only relevant ones. And while Twisted does not allow to run multiple event loops (like the JS run-times mentioned before), asyncio does indeed. That's advanced, but consequent. The fact that Twisted requires explicit reactor start, but then does not allow to start multiple reactors is of course the worst of all I agree. Well. The likeliness that gets fixed is still higher than Python getting rid of the GIL;)

Prevent importing stuff you need all the time and abstract some more

In general, no. IMO too much magic hurts. And auto-importing and aliasing in particular is a great source of hard to track down problems.

Stuff line @inlineCallbacks, @coroutine or returnValue should have wrappers, and the wrappers should be attached either to session (or an object containing session passed instead of session) or connection.

You simply can't write source code that runs unmodified across Py2/3 and tx/aio using co-routine style (the mechanisms under the hood for co-routine style are different). At least I don't know a way, and I haven't seen anything like it. You can write such code only without using co-routine style, using plain deferred/future, and without deferred chaining. This is what AB does internally, and it does that using txaio. Hence, there is no point in trying to alias away @inlineCallbacks and @coroutine.

Check if name == "main" implicitly

No, argument parsing isn't something a WAMP component should be concerned with. It is the startup code that should do that. Plus: when the component is run under CB, there is no point in parsing any command line. If any, a component's custom config is provided to the component by CB.

@meejah
Copy link
Contributor

meejah commented Sep 9, 2015

Another point regarding @coroutine: if you're on Python3 you have to type "yield from" and "return", and on Python 2 it's "yield" and "returnValue"...

@sametmax
Copy link
Contributor

I think we should define the objectives of the API first, because all the proposal seems to tackle different objectives.

  • Level: low level, high level, etc ? E.G : do you think a big project should and will always end up wrapping autobahn for their internal use ?
  • target: people understanding async ? understanding twisted / asyncio ? Both ? Any priority ?
  • one way to do things ? several ways : one for power user, one for new comers ?
  • how much time do you want people (and which people ?) to take for grasping it ?
  • what use case do you want to make obvious ?
  • which level of magic will you allow ?
  • do you want to hide the difference between 2.7 and 3.x, asyncio and twisted ? if yes, to which extend ?
  • what message, what philosophy do you want it to show ?
  • how do you imagine the typical app will be structure on the Python side ? On the JS side ?
  • how do you see it integrating with other components ?
  • on what part exactly are we working ? Only the session part ? Only the connection part ? Both ? More ? What should it do ?

There are no wrong answers to these, only clear statements to make.

@oberstet
Copy link
Contributor Author

@sametmax

Defining objectives first .. absolutely. Makes sense. Here is my (current) view:

  1. API Level: The goal would be a single, recommended high-level API. The API should have a minimal surface, and ideally be shared as much as possible across the different Autobahn's.
  2. Target: User must have entry level knowledge of at least Twisted or asyncio
  3. One way or multiple: There should be one way that works for most use cases
  4. How much time: given the user has entry level knowledge of asynchronous programming and Twisted or asyncio, very little time
  5. What use cases should be obvious: all of the WAMP 4 actions / 2 patterns should be "natural" (principle of least surprise) without reading much/any docs
  6. Level of magic: not a lot (explicit is better than implicit)
  7. Hide differences between Py2/3: Not possible at the indented API level (see txaio for the maximum possible .. but that is too low level for most)
  8. Message: Decoupling. Separation of concerns.
  9. Imagine typical app: the idiomatic app would consist of a set of loosely coupled components, talking over WAMP (where the transport might reduce to function calls when components co-reside in a router/container)
  10. Integration with other components: not sure, you mean eg Django? Answer on that would be not AB, but CB REST Bridge services. Direct integration with blocking, synchronous code is of secondary concern. It just doesn't work nicely / it's always a hack.
  11. On what parts are we working: Connection and Session plus plumbing (the role filled by ApplicationRunner today .. but we could screw that)

@oberstet
Copy link
Contributor Author

I am posting example code resulting from a discussion with @meejah on Slack. This is how a complete working example would look like under that approach:

from twisted.internet import reactor
from autobahn.twisted.wamp import Connection


def main(connection):

    @inlineCallbacks
    def on_join(session):
        res = yield session.call(u'com.example.add2', 2, 3)
        print(res)

    connection.on('join', on_join)


if __name__ == '__main__':
    connection = Connection(main)
    connection.connect(reactor)
    reactor.run()

We've been talking about the design space:

  • subclassing vs composition
  • callbacks vs listeners

What we have today is subclassing (ApplicationSession) + callbacks (ApplicationSession.onJoin etc).

Above is doing composition (it just uses a session) + listeners (connection.on('join', on_join).

Above assumes defaults like @sametmax suggested: WAMP-over-WebSocket connecting to ws://127.0.0.1:8080/ws on realm public, but would allow configurable transports including auto-reconnect and all that by providing appropriate config to the Connection constructor.

Above user code (the stuff inside main()) would run unmodified under CB using a config along these lines:

"components": [
    {
        "type": "function",
        "entrypoint": "myapp.main",
        "realm": "realm1",
        "transports": [
            {
                "type": "websocket",
                "endpoint": {
                    "type": "tcp",
                    "host": "127.0.0.1",
                    "port": 8080
                },
                "url": "ws://127.0.0.1:8080/ws"
            }
        ]
    }
]

I am cautiously optimistic that above would satisfy most of the goals we have. But this needs to be investigated further.

@oberstet
Copy link
Contributor Author

Here is a slight update of the API proposal:

from autobahn.twisted.wamp import Connection

def on_join(session):
    print('session connected: {}'.format(session))
    session.leave()

def main(connection):
    connection.on_join(on_join)

if __init__ == __main__:
    connection = Connection()
    connection.run(main)

Slightly larger example which would run unmodified over Py2/3 and twisted/asyncio apart from changing the imports:

from __future__ import print_function

from txaio.twisted import add_callbacks
from autobahn.twisted.wamp import Connection


def on_join(session):
    result = session.call(u'com.example.myapp.add2', 2, 3)
    add_callbacks(result, print, print)

def main(connection):
    connection.on_join(on_join)


if __init__ == __main__:
    connection = Connection()

    finish = connection.run(main)
    add_callbacks(finish, print, print)

In above, Connection would take an optional reactor or loop argument. When none is given, the default one is used and automatically started in .run(). When a reactor/loop is provided, the user has to start the reactor/loop explicitly. I think this was a suggestion of @sametmax

I am gonna prototype that now and see how it works.

@oberstet
Copy link
Contributor Author

Regarding prerequisite know-how .. there are these areas of knowledge a programmer can have:

  1. synchronous programming (and only that)
  2. using callbacks
  3. deferreds/futures
  4. co-routines

E.g. every JS programmer knows about 2., and 3. is growing in the JS community. ES6 brings native promises to JS. Co-routines seem to be in the cooking for ES7 (see here).

With Python, both Twisted and asyncio propagate not 3. and 4. It is true that a considerable fraction of Python programmers might only know about 1. - but there isn't anything we can do about in AutobahnPython about it.

@sametmax
Copy link
Contributor

It's clean and easier than before, but we need to figure out how to allow a clean deorator syntax version, since right now you end up with:

    from autobahn.twisted.wamp import Connection

    connection = Connection()

    @connection.run
    def main(connection):

        @connection.on_join
        def on_join(session):

            @session.register("foo.bar") # not defined in our previous example
            def _():
                return "foobar"

Let's do this API, but add in the mix an additional object : a registry to collect all callbacks and which will automatically create something like "main()" for you:

class CallbackList:

    def __init__(self):
        self.procedures = []
        self.subcribers = []

    def register(self, name, args, *kwargs):
        def wrapper(func):
            self.procedures.append(name, func, *args, *kwargs)
        return wrapper

    def subscribe(self, topic, args, *kwargs):
        def wrapper(func):
            self.procedures.append(topic, func, *args, *kwargs)
        return wrapper

    # this is the main() equivalent 
    def run_all(self, connection):

        def on_join(session):
            for name, cb, args, kwargs in self.procedures:
                session.register(name, cb, *args, *kwargs)

            for topic, cb, args, kwargs in self.subscribers:
                session.subscribe(topic, cb, *args, *kwargs)

        connection.on_join(on_join)

        return main

Which will allow:

    from autobahn.twisted.wamp import Connection, CallbackList

    connection, cb = Connection(), CallbackList()

    @cb.register("foo.bar") # not defined in our previous example
    def _():
        return "foobar"

    @cb.subscribe("doh") # not defined in our previous example
    def _():
        print('doh')

    connection.run(cb.run_all)

But even then, I'm a bit confused with "run()". We call it open() on JS, so maybe we should call it the same way in Python. And if not, it means they are different and we need to provide JS with run().

@meejah
Copy link
Contributor

meejah commented Sep 14, 2015

There isn't really the equivalent of run in the JS side, because there isn't an event-loop to explicitly start. So, there's still open on both Python and JS, but Python needs to add the concept of run().

Personally, I wouldn't put run on Connection at all -- as that implies that Connection is representing/abstracting two different things: "a WAMP connection", and "how to run a particular event-loop". It's also entirely possible to run several connections in the same Python process with no problem so I would make run a helper-function that can take 1 or more Connection instances as well as logging (and possibly other) options. Like:

from autobahn.twisted import wamp
import example

api = wamp.Connection(example.api.setup)  # on some "public" realm, for example
management = wamp.Connection(example.backend.setup)  # on some private realm, for example

wamp.run([api, management], log_options=dict(level='debug'))

...and a module example/api.py might look like:

from twisted.internet.defer import inlineCallbacks
from autobahn.twisted import wamp

class _HelloWorld(object):
    def __init__(self, db_connection):
        self._db = db_connection

    @wamp.register(u"com.exampe.api.v0.method_one")
    def method_one(self, arg)
        return "Hello, {}".format(arg)

    @wamp.register(u"com.example.api.v0.another_one")
    @inlineCallbacks
    def another_one(self):
        user = yield self._db.query(...)
        other_info = yield self._db.query(...)
        return u"You get the idea."

@inlineCallbacks
def _on_join(db, session):
    api = _HelloWorld(db)
    yield session.register(api, options=RegisterOptions(...))
    yield session.publish(u"com.example.api.v0.ready")

@inlineCallbacks
def setup(connection):
    db = yield connect_to_db(...)
    connection.on('join', _on_join, db)
    connection.on('leave', db.disconnect)

Obviously, you could have several different _HellowWorld-type objects encapsulating different bits of the APIs you want to register, and .register (and .subscribe) them on WAMP connections as appropriate. Notice too that the above class is fairly unit-testable, as you can pass in a fake db object. Also, there are no module-level globals to try and fake out in tests. You can even unit-test the _on_join method fairly easily.

@oberstet
Copy link
Contributor Author

@sametmax

   from autobahn.twisted.wamp import Connection, CallbackList

    connection, cb = Connection(), CallbackList()

    @cb.register("foo.bar") # not defined in our previous example
    def _():
        return "foobar"

    @cb.subscribe("doh") # not defined in our previous example
    def _():
        print('doh')

    connection.run(cb.run_all)

This code can't run unmodified when taken to CB. And it looks very weird to me, and it does too much magic.

But even without taking decorators into consideration, we need to address below ..

@meejah

There isn't really the equivalent of run in the JS side, because there isn't an event-loop to explicitly start. So, there's still open on both Python and JS, but Python needs to add the concept of run().

Yep.

Personally, I wouldn't put run on Connection at all -- as that implies that Connection is representing/abstracting two different things: "a WAMP connection", and "how to run a particular event-loop".

Good point. It's crap to mix these. Back to drawing table:

from autobahn.twisted.wamp import Connection

def on_join(session):
    print('session connected: {}'.format(session))
    session.leave()

def main(connection):
    connection.on_join(on_join)

if __init__ == __main__:
    from twisted.internet import reactor

    connection = Connection()
    connection.open(main)

    reactor.run()

Connection can take a reactor (loop) argument. One can (well, should in Twisted) run multiple event loops in one process.

Connection.open returns a Deferred (Future) that will fire when the connection is really done (won't automatically reconnect, because explicitly closed or max reconnects reached).

Hiding reactor.run just doesn't make sense ..

@sametmax rgd Connection.open vs .start or .run: whatever we name it, I agree, it should be consistent with AutobahnJS.

@oberstet
Copy link
Contributor Author

Maybe we can support both composition and inheritance:

class Connection(object):
    def __init__(self, session_class=ApplicationSession, reactor=None):
         pass

    def open(self, main=None):
         pass

@meejah
Copy link
Contributor

meejah commented Sep 15, 2015

I like the concept of "run" as a simple method (no matter if it's actually called run) as to me it's a lot more intuitive that it just never returns. Maybe I just like it because it mimics twisted.internet.task.react...

@meejah
Copy link
Contributor

meejah commented Sep 15, 2015

@oberstet what about adding a "setup=" kwarg to Connection's init -- and it (if available) would be called with the connection object at some point after we've got a session instance and started the reactor, but before we've joined.

I'm guessing that your main= to open is similar, right? I think that still doesn't handle the use-case of "run multiple connections" though, does it?

class FunStuff(object):
    def __init__(self, db_connection):
        self._db = db_connection

    @wamp.register(u"com.example.meaning")
    def meaning(self):
        return 42

@inlineCallbacks
def connection_setup(connection):
    db = yield connect_to_db()
    api_provider = FunStuff(db)

    @inlineCallbacks
    def joined(session):
        yield session.register(api_provider, options=RegisterOptions())
    connection.on('join', joined)

con0 = Connection(setup=connection_setup)
con1 = Connection(...)

run([con0, con1])

Now, if we can get away with having a "joined" and "left" Deferred/Future as attribute-accessible things on ApplicationSession (which we can, I think, with the caveat that they would need to be re-created every time we disconnect) then we could simplify the above a bit:

class FunStuff(object):
    @wamp.register(u"com.example.meaning")
    def meaning(self):
        return 42

@inlineCallbacks
def main(session):
    db = yield connect_to_db()
    api_provider = FunStuff(db)
    # ^ that's pre-session-join()ed setup stuff (no register, publish, etc)
    yield session.joined
    # now we have a connected session, so we can call register etc.
    yield session.register(api_provider, options=RegisterOptions())
    yield session.publish(u"com.example.ready")
    # now cleanup handlers can run after we've left:
    yield session.left
    yield db.disconnect()

con0 = Connection(main=main)
con1 = Connection(...)

run([con0, con1])

That means: main (if provided) is called with the session object as soon as we have a connection (but before we've joined). So it wouldn't get called, for example, if we haven't ever made a valid connection (server offline, etc). It would have to get re-called whenever we re-join(). I'm ignoring error-handling (e.g. db.disconnect() should be in a finally: block).

@oberstet
Copy link
Contributor Author

Here is the one I am working on master:

from twisted.internet.task import react
from twisted.internet.defer import inlineCallbacks as coroutine
from autobahn.twisted.connection import Connection


def main(reactor, connection):

    @coroutine
    def on_join(session, details):
        print("on_join: {}".format(details))

        def add2(a, b):
            return a + b

        yield session.register(add2, u'com.example.add2')

        try:
            res = yield session.call(u'com.example.add2', 2, 3)
            print("result: {}".format(res))
        except Exception as e:
            print("error: {}".format(e))
        finally:
            session.leave()

    connection.on('join', on_join)


if __name__ == '__main__':

    connection = Connection()
    react(connection.start, [main])

@oberstet
Copy link
Contributor Author

The fun thing is: you can also use inheritance:

from twisted.internet.task import react
from twisted.internet.defer import inlineCallbacks as coroutine
from autobahn.twisted.wamp import Session
from autobahn.twisted.connection import Connection


class MySession(Session):

    def on_join(self, details):
        print("on_join: {}".format(details))


if __name__ == '__main__':

    connection = Connection()
    connection.session = MySession
    react(connection.start)

@oberstet
Copy link
Contributor Author

oberstet commented Dec 1, 2016

and here is yet another try ... how do you like that one?

SUBSCRIBE:


transport = await client.connect(u'wss://example.com')

session = await transport.join(u'example1')

subscription = await session.subscribe(u'com.example.on_hello')

def on_hello(msg, seq, opt=None):
    print('Received {} - {} - {}'.format(msg, seq, opt))

subscription.on(on_hello)

await sleep(60)

await subscription.unsubscribe()

await session.leave()

await transport.disconnect()

@oberstet
Copy link
Contributor Author

oberstet commented Dec 1, 2016

PUBLISH:

transport = await client.connect(u'wss://example.com')

session = await transport.join(u'example1')

seq = 0
while seq < 60:
    session.publish(u'com.example.on_hello', u'Hello, world!', seq, opt=[1, 2, 3])
    await sleep(1)

await session.leave()

await transport.disconnect()

@meejah
Copy link
Contributor

meejah commented Dec 1, 2016

I like the general flow -- and I think a "one-time connect" method is a good idea (vs. e.g. the more "run"/"start" style ones which do re-connection too). (That is, both should be possible)

I'm not sure about the "sub = await subscribe(topic)" followed by the ".on()" thing -- what's the use-case? (i.e. when would you not do the .on() right away). What about allowing decorator-access? (ideally "in addition to" ...). Like so:

session = await ...

@session.subscribe(u'com.example.hello')
def on_hello(name):
    print("hello", name)

@meejah
Copy link
Contributor

meejah commented Dec 2, 2016

Further to the decorator idea, via the existing Component:

component = wamp.Component(...)

@component.on_join
async def connect_to_db(session, details):
    db = await txpostgres.Connection().connect(...)

(Similar for the other events you can subscribe to on Component of course).

@oberstet oberstet removed this from the 0.11.0 milestone Mar 18, 2017
@oberstet
Copy link
Contributor Author

I think the discussion is heading towards consensus (inheritance = bad, observers = good, ..). One thing though is to define/document the events that can be observed on a session.

Eg., currently we have these https://github.com/crossbario/autobahn-python/blob/master/autobahn/wamp/component.py#L398 events:

  1. start
  2. connect: fired when a transport is connected to the session (the transport itself must be itself "connected" at the network level to be of use to the session of course)
  3. join: fired when the WAMP opening handshake is completed (HELLO -> CHALLENGE -> AUTHENTICATE -> WELCOME)
  4. ready: fired when ALL observers on "join" have finished
  5. leave
  6. disconnect: wired when the transport is disconnected from the session (the transport itself might stay "connected" at the network level to be reused with the same or a different session)

@meejah what was start for again?

also, we only have leave, not left (or similar) - does it make sense to distinguish between "right before WAMP closing handshake is started" and "ALL former "leave" user observers are done and the WAMP closing handshake is finished."

@oberstet
Copy link
Contributor Author

So a transport is "connected" at the network level (TCP connection is established), and then "connected" to a session.

"connected" is hence overloaded .. but we could also go with:

  • a transport is connected on the underlying network
  • a transport is attached to a session

Any preferences?

@meejah
Copy link
Contributor

meejah commented Jun 12, 2017

I think "start" is just fired when you "started running your Component" .. but yes is should probably have a matching "done" or "stopped" or similar if we want it?

I think "join" / "leave" and "connect" / "disconnect" are the right pairs of words? (or it would want to be "joined" and "left").

IIRC we had semantics that the future returned from Component.start() would fire when the thing was "done" (which is after all on-leave listeners are completed) so that points to having a "done" event mirroring "ready" (i.e. "ready" fires after join + all listeners, "done" fires after "leave" and all listeners)...but looking at it now maybe that would be more clear if the events were "join", "joined" and "leave", "left"? And then keep "connect" / "disconnect".

Looking in the code, "start" currently fires as soon as something calls .start on a Component .. so I guess a matching "end" event would probably make sense too (which fires at the same time as the future returned from Component.start).

Then the overall picture would be something like: start -> connected -> join -> joined/ready -> leave -> left -> disconnected -> end. The "connected -> ... -> disconnected" part would be a loop while doing retrys or re-connects and so "end" would only fire after we'd given up on all transports and retry/reconnect attempts (or were told stop).

@meejah
Copy link
Contributor

meejah commented Jun 12, 2017

I think saying "a Session is attached to a transport" (or I guess "a transport is attached to a session") makes the most sense and is least ambiguous.

@oberstet
Copy link
Contributor Author

subseded/fixed in #964

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

No branches or pull requests

5 participants