crossbario / autobahn-python

WebSocket and WAMP in Python for Twisted and asyncio
https://crossbar.io/autobahn
MIT License
2.48k stars 766 forks source link

App wrapper on top of ApplicationRunner #208

Closed oberstet closed 6 years ago

oberstet commented 10 years ago

From: https://groups.google.com/forum/#!topic/autobahnws/lmztderi6N4

What I think is the biggest adoption issue, however, is the API. Autobahn should have a small cost of entry, and right now it doesnt. The hello world is not only hard to write using the documentation, but quite scary :

  from  twisted.python  import  log
  from  twisted.internet.defer  import  inlineCallbacks

  from  autobahn.twisted.wamp  import  ApplicationSession
  from  autobahn.twisted.wamp  import  ApplicationRunner

  class  ListenForEvent(ApplicationSession):

       def  __init__(self, config):
           ApplicationSession.__init__(self)
           self.config  = config

       def  onConnect(self):
           self.join(self.config.realm)

       @inlineCallbacks
       def  onJoin(self, details):
           callback =lambda  x: log.msg("Received event %s"  %  x)
           yield  self.subscribe(callback,'un_evenement')

  if  __name__ =='__main__':
      runner = ApplicationRunner(endpoint="tcp:127.0.0.1:8080",
                                 url="ws://localhost:8080/ws",
                                 realm="realm1")
      runner.run(ListenForEvent)

We need a simplified wrapper for that, that can be used for simple use case : small applications, tests, etc. In the same fashion that flask and bottle make web programming much easier than Django. I'm suggesting to add something like this :

  from  autobahn.app  import  App

  app = App(url="ws://localhost:8080/ws")

  @event("event_name")
  def  handle(details):
       app.log("Received event %s"  %  details)

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

Sane defaults, doesn't not cover all the use cases, but it's enought for 80% of the dev. The goal is not to replace the other API, but rather offer something higher level we can show of in the first page of the documentation that would make people started quickly.

halflings commented 10 years ago

After reading the article on SamEtMax, I started working on such a wrapper and would love to have feedback. Currently implemented it this way:

from twisted.internet.defer import inlineCallbacks

from autobahn.twisted.wamp import ApplicationSession
from autobahn.twisted.wamp import ApplicationRunner

class App(object):
    def __init__(self, *args, **kwargs):
        self.application_runner = ApplicationRunner(*args, **kwargs)

        class SessionClass(ApplicationSession):
            def __init__(self, config):
                super(SessionClass, self).__init__()
                self.config = config

        self.session_class = SessionClass

    def event(self, event):
        """Decorator for events callbacks."""

        def decorator(callback):
            @inlineCallbacks
            def on_event(self, details):
                yield self.subscribe(callback, event)
            func_name = 'on{}'.format(event.capitalize())
            setattr(self.session_class, func_name, on_event)
            return callback

        return decorator

    def run(self):
        self.application_runner.run(self.session_class)

Would something like this be enough or is it too hacky? (particularly the part where I add the on_event function as an attribute, there are many cases we'd have to cover to generate a right name for the function, as events can have special characters).

I don't know how this works exactly, I feel like all "onSomething" functions are currently considered as functions that subscribe to an event, so if that's the case we could also just generate any random name beginning with on (and we could improve that later by bypassing this mechanism)

sametmax commented 10 years ago

There are things to consider :

A great, simple, yet powerful API is always a lot of work. This is why most people don't do it :)

oberstet commented 10 years ago

I'll add more comments, but here is working code:

https://github.com/tavendo/AutobahnPython/blob/app_wrapper/examples/twisted/wamp/app/hello.py https://github.com/tavendo/AutobahnPython/tree/app_wrapper

The "hello world" example above has exactly same number lines of codes as "hello world" in Flask;) It doesn't get much simpler ..

oberstet commented 10 years ago

@halflings the app.procedure and app.event decorators run before the session is even established and hence need to only buffer the info. Later, when the app session is there, the buffered info needs to be applied.

oberstet commented 10 years ago

@sametmax

Making run() accept an "event loop" (that is defer the choice of Twisted/asyncio) till then: I don't think this approach works. When you want to yield within or return Deferreds from user code (which you definitely want), you have to make a choice anyway before.

A user must decide writing his code:

E.g. yield from is valid syntax only on Python 3.4+. There is no way to make such code run under < 3.4.

The reason Autobahn is able to support all of those using one codebase is a combination of tricks .. part of it being: not using co-routines at all internally (only Futures/Deferreds and mixing those in). This is ok for library code, but I think too big a burden for user code ..

halflings commented 10 years ago

@oberstet I was actually adding the info to the class (as a method) and not to the session's instance. But the way you've done that is way cleaner (I didn't understand that you could subscribe to different procedures in onJoin!)

oberstet commented 10 years ago

@halflings Ah, ok. Wasn't looking hard enough;)

Btw: you can register (and unregister) procedures not only in onJoin, but any time (as long as the session is still established). Same for event handlers (subscribe and unsubscribe).

oberstet commented 10 years ago

@sametmax It's a looong list;) I don't understand everything .. in any case, I guess we'll need to approach that step by step ..

sametmax commented 10 years ago

@oberstet good point, the event loop one. I guess we will need 3 apps module then.

sametmax commented 10 years ago

Hi,

I couldn't sleep much tonigh, so I took your codes and pushed it a little further.

http://0bin.net/paste/N-q9vTEWdo-TN70H#UgqZbdHWG02on5LC4h56nGw+B+xZeFD6ayv4ycm/2uk=

Now :

Everything seems to work, also I need help with these things:

After adding tests, i'll make a PR so you can peer review the code more confortably. Do not hesitate to iterate on it in the mean time. I like where this is going.

BTW, I'm currently working from Thailand, so I got a +5h time difference, this explains the weird presence hours.

oberstet commented 10 years ago

Hi Michel,

firstly, it seems you now got familar with things enough to hack around and try things as you like. Guess that is good;)

Some comments:

Yes, by default, a publisher (that is itself subscribed to the topic published to) does not get the published event himself. This is by design, but can be overridden via options = PublishOptions(excludeMe = False)

Application should not run an embedded router. In fact, it should not know anything about how to host/run, but only define application functionality (the what, not the how). A user should be free to choose a hosting environment for WAMP application components without ever changing his code, and without changing AutobahnPython internals.

The decorators for registering and subscribing should be better left called procedure and topic, since that is what they describe: URIs for the respective code (not the action of hooking up). We've been trying hard to establish terminology: "procedure" (= URI) under which an "endpoint" (code) can be called remotely. "register" = act of hooking up a "procedure" at a router.

Auto-adding decorators for co-routines: there are 2 problems. First, it doesn't seem to work https://gist.github.com/oberstet/4ea003fd8e91dcb9cba3. Second, I'm not convinced of overly "magic" things. It can make understanding things harder. A user can attach any number of decorators to a single function, so that works out of the box and makes things clear and obvious.

Replicating call and publish (forwarding to ApplicationSession) does not seem to add real value. Of course, then, the user should have access to the ApplicationSession and can use that for the respective functions.

Right now, the only officially supported API for WAMP application code is defined in the following interfaces: ISession, ICaller, ICallee, IPublisher and ISubscriber. Pls see: https://github.com/tavendo/AutobahnPython/blob/master/autobahn/autobahn/wamp/interfaces.py#L271

If something Application like should get official support, it must be (first) defined via an interface. This is probably one of the most important issues. API stability, in particular for WAMP application code. And the way we say "this is official API, and everything else not - use at your own risk - might break any time" is: defining interfaces (Python ABCs).

Auto-magically creating an URI from module/func names: for me, this is confusing stuff. The URI of a procedure endpoint or topic event handler should be defined by the user exclicitly - always. In particular, since URIs will soon be able to be "patterns" also (WAMP advanced profile). An URI like mymod1.myfunc1 also isn't what we recommend (see the WAMP spec). URIs should be like com.mycorp1.myapp1.myfunc1. One reason for this recommended namespacing is to be able to combine app components from different "vendors" into 1 overall application without naming conflicts.

"Lifecycle events:" for me, the names "event" and "signal" suggest that the stuff can happen more than once during a session. This is not the case with joining or leaving a realm. Those are pretty much the begin and end of life of a single session. Of course users should be able to hook into those. But I don't think it should be called "event"/"signal" and it shouldn't be exposed via decorators.

I guess we need more "design" and experimentation at this stage. In particular, since if we add anything else but the current official API for WAMP application code, it'll need to be supported and stable.

sametmax commented 10 years ago

Le 28/05/2014 14:03, Tobias Oberstein a écrit :

Hi Michel,

firstly, it seems you now got familar with things enough to hack around and try things as you like. Guess that is good;)

It means I can start to participate instead of just harrassing you on the mailling list :)

Some comments:

Yes, by default, a publisher (that is itself subscribed to the topic published to) does not get the published event himself. This is by design, but can be overridden via |options = PublishOptions(excludeMe = False)|

Ok. You already told my that for the browser client, so sorry for making you repeat yourself. I'm a bit stupid sometime.

The rest of your observations are very much the old battle of correct VS practical we see in the Python community (Django VS Zope for example). In the end, I will always fix it the way you tell me to because acceptance is more important than being right, but still, I want to defend this design since I'm a ergonomist and API conception is pretty dear to me.

|Application| should not run an embedded router. In fact, it should not know anything about /how/ to host/run, but only define application functionality.

Correct : separation of concerns and uncoupling.

Practical : a new user don't have to have to learn about an external script to run it as a server. For you it may not seems much, but it's the additionsof little things like that that make a tool easy to use or not. Indeed, bottle or flask DO includes a server in their run method. You could claim that their are server side only technologies, and again, that's correct, but you break the principle of least suprise by not allowing your app to work out of the box when you call python myscript.py.

I litterally spent days to understand WAMP + autobahn + crossbar, and only because I was looking for something like this for a long time. Very few people will have this will to do so. Mostly, they will land on the doc first page, pip install (if it fails because of lacking of python-dev or gcc, they will leave without looking for the cause, so we need to fix that has well, probably upstream in twisted), then copy / paste the exemple code and run it. If it doesn't work, half of them won't even bother looking for instructions around the example snippet to understand they need to start a server. This is the web of today : lots of projects claiming they the new hot thing, very few time to actually check it's true.

The decorators for registering and subscribing should be called |procedure| and |topic|, since that is what they describe: URIs for the respective code (not the action of hooking up).

Correct : decorators describe what they are applied to.

Practical : decorators are named after the existing API to avoid multiplication of the technical jargon, lowering the entry cost. What is a procedure ? What is a topic ? You need to read the doc to know. You already know what register/subscribe means, and even if you don't, you have only to learn what they do once, and when you see session.register/subscribe, you already know what it does. Less things to learn. Very important.

Auto-adding decorators for co-routines: there are 2 problems. First, it doesn't seem to work https://gist.github.com/oberstet/4ea003fd8e91dcb9cba3.

It does work, it's just that @inlineCallback wraps the login so it returns a deferred, breaking introspection. The idea is that people won't even know about inlineCallbacks (they should not have to know anyway), so they won't use it.

Second, I'm not convinced of overly "magic" things. It can make understanding things harder. A user can attach any number of decorators to a single function, so that works out of the box and makes things clear and obvious.

Correct : "import this" => explicit is better than implicit.

Practical : this is a boiler plate line to write for every function that has nothing to do with your work : it's the underlying implementation leaking (since it would be @coroutine for asyncio...). It is forcing people to know about the implementation details AND to write boiler plate about it. Plus, if people forget about it, the callback seem to do nothing, no error is raised. Just nothing happens. Aagin, this makes the entry cost higher.

But the more important thing is : decorator order matters. And most people don't know that because most people don't know how decorator works. So they will apply @inlineCallbacks on the top of another decorator, and it will not work. And they will have not clue why because it will not raise any meaningful error. And they will get frustrated, either flooding the mailing list (wasting your time and theirs), giving up WAMP, or find out by themself ofter hours of irritating trials/errors. You know this feeling, the feeling somebody didn't make you life easier while they could and destroy your afternoon.

Replicating |call| and |publish| (forwarding to |ApplicationSession|) does not seem to add any value. The user should have access to the |ApplicationSession| and use that for the respective functions.

Correct : "import this" => there should be only one way to do it. DRY.

Practical : calling details.app.session.publish is ugly, makes code readability harder, leak app implementation (people suddenly need to learn about sessions, what good is that to them for a small site ?) and prevent shell discoverability via autocompletion, dir() or help(). These are very comon actions, there should be easy to do, and easy to discover without fuzz. When you play with the exceptionnaly designed requests lib, you don't need to look in the doc for easy things. requests doesn't force you to call the real underlying method handlers, it exposes them at the root level if it's very common, so you can fire iPython and get started.

Right now, the only officially supported API for WAMP application code is defined in the following interfaces: |ISession|, |ICaller|, |ICallee|, |IPublisher| and |ISubscriber|. Pls see: https://github.com/tavendo/AutobahnPython/blob/master/autobahn/autobahn/wamp/interfaces.py#L271

If something |Application| like should get official support, it must be (first) defined via an interface.

I see. This is very heavy requirement. You usually see that in Java project, rarely in Python projects. I do see the value for the core components, so people can create their own, reassemble them, and still have the garanty they are compatible.

However, it seems overkill for Application IMO : it's not a reusable component. It's not a core composable objet. There is few value in subclassing it. It's a dump API to get your started.

Auto-magically creating an URI from module/func names: IMHO, this is confusing stuff. The URI of a procedure endpoint or topic event handler should be defined by the user exclicitly. In particular, since URIs will soon be able to be "patterns" also (WAMP advanced profile).

Correct : "import this" => explicit is better than implicit.

Practical : the typical web application written with bottle got 10 to 20 methods, top. It's not a huge system with dozens of components talking to each other. I know this is WAMP final goal, but nobody starts by writting a big app with a new tech. You typically give it a try with small one to see how it goes and not invest too much. You should always make easy thing easy (here, avoiding repeating the function name), and hard thing possible (in the rare case of name conflict, allow an override).

An URI like |mymod1.myfunc1| also isn't what we recommend (see the WAMP spec). URIs should be like |com.mycorp1.myapp1.myfunc1|.

Correct : follow the spec, use a corporate well known solution

Practical : This suppose all projects have a Website. With pypi, readthedoc and github covering all your needs, many projects now don't. Some project runs on several websites and none is the offiial. Some are running on intranet. Some, you don't want to expose the URL they are attach to for political reason. But more over, Python modules already are namespace. You can't get two Python modules with the same name uploaded to pypi or installed in your virtualenv, so using module.function is most probably unique. Nothing prevents power users to use the serious naming convention for big project though.

"Lifecycle events:" for me, the names "event" and "signal" suggest that the stuff can happen more than once during a session. This is not the case with joining or leaving a realm. Those are pretty much the begin and end of life of a single session. Of course users should be able to hook into those. But I don't think it should be called "event"/"signal" and it shouldn't be exposed via decorators.

Correct : use name that suggest it will be called only once.

Practical : Django and QT already use "signals" and "events" to talk about this concept. People from these projects (which are ver popular) will already know what we are talking about, lowering the entry cost. This is the purpose of the Application object, lowering the entry cost. It does mean rouding edges, removing a thousand of paper cuts. Besides, You may will to add and "event_received" signals eventually, which will be called for each PUB/SUB events to allow the user to make code called for all event and creates middlewares. This will be called several times.

I guess this all needs more "design" and experimentation first. In particular, since if we add anything else but the current official API for WAMP application code, it'll need to be supported and stable.

— Reply to this email directly or view it on GitHub https://github.com/tavendo/AutobahnPython/issues/208#issuecomment-44372188.

Or course. This is why I wish you haven't have closed this issue so we can carry on the debate.

I also need some clue on how to write tests for it.

How do you want the next proposal to be made ? As pull requests so you can fix the code directly ?

What's more, if this is too disruptive, there is alway the option to create an external project on top of autobahn and list it on the documentation. It can feature an opiniated API without your team having to maintain it, or pay the price for the experimental design. You would still benefit from it by just saying in the doc "if you are new to WAMP, you can try this lib first to get a taste of it".

There are a lot of things that need to be done to make it a workable solution :

Regards

oberstet commented 10 years ago

Preface: I will not close this issue, and answer in detail on everything / continue discussing. I just need to get stuff done ..

In the meantime, how about this (works already):

oberstet@corei7ub1310:~/temp$ crossbar init --template hello:python
Creating directory /home/oberstet/temp/.crossbar
Creating directory /home/oberstet/temp/hello
Creating file      /home/oberstet/temp/setup.py
Creating file      /home/oberstet/temp/MANIFEST.in
Creating file      /home/oberstet/temp/README.md
Creating file      /home/oberstet/temp/.crossbar/config.json
Creating directory /home/oberstet/temp/hello/web
Creating file      /home/oberstet/temp/hello/__init__.py
Creating file      /home/oberstet/temp/hello/hello.py
Creating file      /home/oberstet/temp/hello/web/index.html
Creating file      /home/oberstet/temp/hello/web/autobahn.min.js
Node initialized

To start your node, run 'crossbar start'
oberstet@corei7ub1310:~/temp$ crossbar start
2014-05-28 15:22:53+0200 [Controller   4531] Log opened.
2014-05-28 15:22:53+0200 [Controller   4531] ============================== Crossbar.io ==============================

2014-05-28 15:22:53+0200 [Controller   4531] Crossbar.io 0.9.4 starting
2014-05-28 15:22:54+0200 [Controller   4531] Running on CPython using EPollReactor reactor
2014-05-28 15:22:54+0200 [Controller   4531] Starting from node directory /home/oberstet/temp/.crossbar
...
oberstet commented 10 years ago

That means, a Python developer can start by doing:

pip install crossbar
crossbar init --template hello:python
crossbar start

To be honest, I can't see how we can make it much easier;)

Under the hood, there is now a complete "app/node" templating thingy, e.g. the "hello" above is created from template via Jinja2 : https://github.com/crossbario/crossbar/tree/evil_issue/crossbar/crossbar/templates/python

oberstet commented 10 years ago

Some of the issues you raise above are already implemented (upcoming release of Crossbar):

sametmax commented 10 years ago

Le 28/05/2014 20:35, Tobias Oberstein a écrit :

Some of the issues you raise above are already implemented (upcoming release of Crossbar):

  • install without any compiler (relevant dependencies are now all "soft")
  • better app dev. log output
  • forwarding Python tracebacks within WAMP error messages to client (debug backend from frontend)

Great !

  • reloading and restarting of Python app classes without restarting Crossbar

Do you mean we can actually hot swap Python code into crossbar ????? This is huge.

  • auto-doing the latter (via filesystem watcher) - this is not yet done, but will be in the release

— Reply to this email directly or view it on GitHub https://github.com/tavendo/AutobahnPython/issues/208#issuecomment-44406778.

Good. Filesystem watchers can also build useful for your future users to automatically use compilers/minifier/preprocessor during developpement. With time, a fantastic dev ecosystem can be built out of this. Background test runners. Cron jobs. Code quality checks. The potential of this is enormous.

About the --init options, it's a good feature (so good django added it 2 versions ago), but it solves a different problem : it makes it easy for somebody who knows what he is doing to start off. And it make it easy for somedy who doesn't to explore. But it doesn't force us to make the API easy to grasp. You can have the best templates in the world, if your API is not intuitive, people will struggle.

This is why Django won over Zope, Turbogear and Pyramid. And this is why now flask is attracking a good number of Django users.

About the server in Application issue, we can actually make two Application classes, one without it, and one child class embedding it all. We can make an Application Interface so people can build application with more usages in mind : HTTP, SSH, FTP, etc. Since we got twisted under the hood, we can leverage it's power.

To finish a a funny note :

http://blogs.hbr.org/2014/05/why-germany-dominates-the-u-s-in-innovation/

oberstet commented 10 years ago

hot swapping code: yes, the core is here https://github.com/crossbario/crossbar/blob/evil_issue/crossbar/crossbar/common/reloader.py#L56

and it works like this:

  1. developer edits his class derived of ApplicationSession.
  2. Crossbar has previously noted the underlying file, and all modules loaded from there (recursively)
  3. Crossbar triggers reloading of all changed modules (not only the file with the entry class)
  4. Crossbar the restarts the component only (the worker or router in which the component runs is not restarted - at least not required to)#

You might note the "limitation" that the app session MUST be reestablished.

This is inevitable with Python, since there are no hooks for hot migrating instances of the old class to instances of the new class.

As said: we cannot do anything about this. It's a Python restriction. With say system like Erlang/OTP, you have machinery built right into the run-time for real hot code - including migrating "instances".

But apart from that: yes, it's nifty.

oberstet commented 10 years ago

rgd. the blog post: =) sure, but rgd software, Europe sadly doesn't have that much lever .. which I acknowledge does somewhat hurt me. I'd like to avoid moving to SF;)

there is this "new thing": IoT. this will be huge. and the claims aren't set yet. lets try ...

sametmax commented 10 years ago

Indeed, espacially since now we got microPython now. And rasberrypy. And WAMP on c++.

oberstet commented 10 years ago

@sametmax I have started a Wiki page on "contributing" https://github.com/tavendo/AutobahnPython/wiki/Contributing

In particular it describes the technical side of the workflow for contributing:

https://github.com/tavendo/AutobahnPython/wiki/Contributing#code

This workflow wasn't necessarily followed in the past, but I think it's important as we gain momentum and must maintain a robust code basis.

The workflow is nothing esoteric .. it's one "best practice" that projects follow.

If you wanna try it, find a simple documentation issue you see and proceed the workflow. That way, we can check the workflow on easy/uncontroversial stuff first.

oberstet commented 10 years ago

@sametmax rgd. interfaces: there are 2 purposes

1) "reusable components" .. along the lines of this answer http://stackoverflow.com/questions/3570796/why-use-abstract-base-classes-in-python and http://legacy.python.org/dev/peps/pep-3119/#id31

This probably doesn't apply to Application (or ApplicationSession), since it's unlikely to get alternative implementations.

2) Documentation and clear "interface contract & promise".

We need to be able to point users to a "stable API". Which is the API they use for implementing their user code.

Python allows of course to do anyone access and modify anything. However, I'd like to be able to say: look, there are these interfaces. If you use those, and only those, we "guarantee" stability. If you use anything else, you are at your own perils.

Interfaces are a way for documenting this in a formalized way. Otherwise we would need to add some note to docstrings. Cannot be automated etc.

The convention of "everything not starting with _ is public" is "not good enough" for me.

Btw: we've already migrated interfaces from Zope.interface to Python standard ABCs. Those are avail. everywhere.

I'm not willing to compromise on this. Interfaces are required for any user API thing. If it's not there, it's a bug. If we add something, it MUST have an interface defined first.

sametmax commented 10 years ago

Ok.

About the other points, I need a feedback too. And eventually a definitive GO / NO GO as I'm going to write tests, the interface and documentation, and changin things after that is going to be much more work.

Also, if you feel the App is going to take too much time (I understand you are swamped and maybe currently unable to spend the time required for an external contribution) or be too disruptive, we can still make it a separate lib.

I can do something, but I need to know what to do.

Talking about that, how is it going with the article ? Do you need help with the reformating ?

oberstet commented 10 years ago

Ok, please start work on this on the "issue208" branch https://github.com/tavendo/AutobahnPython/blob/issue208/autobahn/autobahn/twisted/wamp.py#L176

I writing an interface for Application and address some you the points you raised in the docstrings. And the other questions here.

oberstet commented 10 years ago

Ah, rgd. the article: no, thank you, Alex will address that soon.

Regarding "embedded router" in Application: ok, lets do that when Application is run in "development mode" (hence, we need such option/flag to Application).

Decorators: Ok, makes sense. Naming them after the existing procedural API (@subscribe and @register). Then we should renamed the decorators here too https://github.com/tavendo/AutobahnPython/blob/master/autobahn/autobahn/wamp/__init__.py#L26

I add a separate issue for that https://github.com/tavendo/AutobahnPython/issues/221, since e.g. examples need also be adjusted

oberstet commented 10 years ago

Rgd "replicating call/publish/.." on Application: ok, then we'll make that explicit in that Application will itself be a provider of https://github.com/tavendo/AutobahnPython/blob/master/autobahn/autobahn/wamp/interfaces.py#L351

Rgd "auto-naming" and URI: if we give the user the option to set a "base URI" for the Application, the decorators can just @register or @subscribe and the function name being appended to "base URI" automatically. Lets do that.

Signals/lifecycle events: ok, lets use a scheme like @signal("onjoin"), @signal("onleave") ..

sametmax commented 10 years ago

Ok, got a branch issue208.

Regarding "embedded router" in Application: ok, lets do that when Application is run in "development mode" (hence, we need such option/flag to Application).

Good idea. Much like runserver in Django, then advertise the need for crossbar in production.

BTW: we can't do this for asyncio/trollus, so they won't implement IProvider.

Rgd "auto-naming" and URI: if we give the user the option to set a "base URI" for the Application, the decorators can just @register or @subscribe and the function name being appended to "base URI" automatically. Lets do that.

Ok. Currently flask and a like gives a name parameter to their app, and usually People pass a name to it. Making the name required garanty us a namespace. I'm going to add that.

Signals/lifecycle events: ok, lets use a scheme like @signal("onjoin"), @signal("onleave") ..

Ok, I'm getting rid of the "_".

Regarding the naming convention, I not sure what style to use to be congruent with autobahn style. It's not PEP8 (since you got " = " in method calls instead of "=" and camelcase method names) but sometime it is (some attributes are snakecase). PEP8 recommand to follow the style of the current project for congruency instead of trying to apply its guidelines, so I'm going to write code using your style (which seems to be twisted style). Is there a document for the code style in this project (if not, we may want to open a ticket to work add something to https://github.com/tavendo/AutobahnPython/wiki/Contributing) ? Do you have any particular wishes about it ?

sametmax commented 10 years ago

We have a working Application for twisted :

https://github.com/sametmax/AutobahnPython/blob/issue208/autobahn/autobahn/twisted/app.py

It now needs the Interface, the unit tests and the documentation.

I will work on them once you reviewed the current code.

oberstet commented 10 years ago

I'll review that soon, but my main problems are more fundamental - we need to have answers for those:

sametmax commented 10 years ago

Le 01/06/2014 22:09, Tobias Oberstein a écrit :

I'll review that soon, but my main problems are more fundamental - we need to have answers for those:

  • With |ApplicationSession| we have an official API for writing application components. |Application| would be a 2nd.
  • I cannot write my app code using |Application| and later host it in Crossbar
  • Why would I use raw Autobahn to write applications anyway when there is Crossbar
  • the basic router included with Autobahn does not implement the advanced profile from WAMP. Which means, certain features cannot be used, only when running Crossbar as router.

— Reply to this email directly or view it on GitHub https://github.com/tavendo/AutobahnPython/issues/208#issuecomment-44779939.

My previous commit made sure you can run the application in crossbar. You use the app instance like an ordinary component :

           "type": "container",
           "component":  {
              "type": "class",
              "name": "your_module.app_instance"
           },

The app expose the underlying ApplicationSession object via the call method in order to do that.

The next important thing is to make sure you can also easily add an app in a twisted reactor next to pure twisted callbacks. This way people from others project such as cyclone can use it next to their current code.

I'll repeat that if you are unconfortable with the Application, it is absolutely OK to make it separate project. But I do believe it will make your "getting started" docs much sexier.

The app concept is way more flexible that it's usually though to be, partly because most implementations are very monolitic.

Here is a blog post I have wrote about the potential of crossbar and what you could do by writting a framework on top of it with this kind of syntaxic style:

http://sametmax.com/le-potentiel-de-wamp-autobahn-et-crossbar-io/

While I don't think it this kind of thing should live in the crossbar repo, I do think having a glimpse of that with Application can motivate people to build awesome tools with your software.

oberstet commented 10 years ago

Ok, I am looking at the code, playing a little. there are multiple issues.

The use of Context changes the API of every procedure/handler. It cannot be any longer used with the stuff we have now (session.register() won't provide a Context) - and it doesn't add value. This needs to be removed. Means, the following should be valid:

@app.register('com.example.add2')
def add2(a, b):
   return a + b

@app.register('com.example.hello')
def hello():
   res = yield app.session.call('com.example.add2', 2, 3)
   returnValue("Hello: {}".format(res))

Writing app.session.call seems to be ok. We shouldn't even try to replicate the API on session on app (which would allow app.call .. insignificant).

Also: Context tries to be "smart" and hides PublishOptions. Nope. Does change API. Using Application instead of ApplicationSession should not require the developer to learn new API.

oberstet commented 10 years ago

The signals for onregister and onsubscribe fire when the decorators run, not when the procedure/handler actually is registered/subscribed, neither when the latter was indeed successful. What was your idea with the signals here? When should they fire?

The signal onrun won't fire when running on Crossbar. Crossbar will only call app.__call__ to produce a session, and do nothing else on app.

oberstet commented 10 years ago

I've merged your code and adjusted it further here: https://github.com/tavendo/AutobahnPython/tree/issue208 This works with https://github.com/tavendo/AutobahnPython/blob/issue208/examples/twisted/wamp/app/hello.py

sametmax commented 10 years ago
The use of Context changes the API of every procedure/handler. It cannot be any longer used with the stuff we have now (session.register() won't provide a Context) - and it doesn't add value. This needs to be removed.

Ok, backward compat is more important than convenience. Context was actually meant to carry more things later, such as informations about the message sender (identity, permissions, etc), but we can do without it for now.

Plus, good catch with returnValue, I completely forgot about that.

Writing app.session.call seems to be ok. We shouldn't even try to replicate the API on session on app (which would allow app.call .. insignificant).

It's not insignificant. Little things like that adds up, leading to decrease the overall cognitive charge of using your tool. Now, there is no benefit to more debate, I prefer that we land something you are absolutely comfortable with.

The signals for onregister and onsubscribe fire when the decorators run, not when the procedure/handler actually is registered/subscribed, neither when the latter was indeed successful. What was your idea with the signals here? When should they fire?

This is bad naming on my part. Something like "onaddrpccallback" is more appropriate, then we should indeed add "onregister", "onregisterfails" to onJoin(). The idea behind "onaddregistercallback" is to have a hook for you to modify the function of a 3rd party code registering callback. Now I realize, because I didn't pass a mutable object wrapping them or forcing signal handlers to return values, what you can do with it is somehow limited (E.G: you can't decorate them).

It should be enough for now, but we may want to improve that in the future.

I'll go ahead and fix that and correct some the docstrings as well.

The signal onrun won't fire when running on Crossbar. Crossbar will only call app.__call__ to produce a session, and do nothing else on app.

Didn't think about that. It's useless, removing it is the right thing to do. Dev will plug on "onjoin"

oberstet commented 10 years ago

One more trivia: could you add a copyright header to app.py? I didn't do that, since the file contains code written by you, and you, as a contributor, should make a explicit decision that is tracked in the repo. Actual authorship is tracked in the repo anyway, so I'd prefer you just copy over the header verbatim from some existing file.

Regarding Context: there are 2 kind of information a "free standing" procedure would like to get access to:

  1. the session on which it is called (which is exactly the one on which it was registered)
  2. any call details

Now, with 1), we need to think about how Application is supposed to be used. Can I call __call__ multiple times to create multiple sessions?

If so, how will that work? In this case, I could see a need to forward the specific session to the invoked procedure as a parameter.

If not, there can be (at most) one session associated with an Application object - and then I can access that via the Application object itself. Means: app.session.call - or, if we replicate API, app.call.

With 2), currently this works like this:

def square(val, details = None):
   ...

self.register(square, 'com.myapp.square', RegisterOptions(details_arg = 'details'))

During registering, you announce your wish to receive call details, and receive those in details named keyword argument.

This feature isn't exposed via decorators currently. But it could be:

@app.register('com.myapp.square', details_arg = 'details')
def square(val, details):
   ...

Sidenote: details_args name "looks" terrible. I am ready to do a review of the whole public facing API for naming things like that ..

oberstet commented 10 years ago

@sametmax any progress on this? would be nice if we could land a first iteration of this with the next release ...

sametmax commented 10 years ago

Hello @oberstet. I haven't forgot, I'm currently working in Asia, sometime I got a lot of time in my hands (like some weeks ago), sometime I got a load of work at once (like right now). I'm not off the project, I actually plan to finish this, and write some tutorial because lots of people signaled their interest but didn't know where to start.

oberstet commented 10 years ago

@sametmax great! let me know if you need any input. would be great to land this with the next AutobahnPython release ..

sametmax commented 10 years ago

New commit :

https://github.com/sametmax/AutobahnPython/blob/issue208/autobahn/autobahn/twisted/app.py

In summary :

Added the copyright.

Fixed and improved lots of docstrings. Tested examples.

Signals for registering/subcribing callbacks are now moved to onJoin(). The semantics is better this way

I edited the run() method :

url now default to "ws://localhost:8080". Most dev servers (including Django, flask and bottle) have such default, so people expect it. For the same reason, I added a print (not a log) of the current url.

realm now has a default value of "realm1". This makes it work out of the box with all examples, and let people play with autobahn without having to wonder what a realm is for. We can always introduce them to that later in the documentation.

standalone has now a default value of True. Indeed, you usually will call run to get a working application, including the dev server. If you want False, most of the time it means you should run it in crossbar anyway. For the few cases when you want False and not use crossbar (like dev two component talking to each others, but don't want to setup the whole crossbar), then you can just pass it to False manually.

Edited subscribe() to remove the automatic event naming.

Indeed, while it makes sense to automatically create the name of a remote procedure you provides, it doesn't make sense to automatically create the name of an event you may or may not be providing, but you are currently telling you will be reacting to.


I'm not sure we should swallow exceptions in signals handlers. You would then run some buggy code without knowing it if you don't thing about checking the log on production.

Once we got this part right, can you implement the interface ? I don't feel comfortable doing that. After that, I can start making unit tests, and doc, then port the work to asyncio and trollius (minus the embeded server).

.

oberstet commented 10 years ago

@sametmax Thanks! I have moved the code to https://github.com/tavendo/AutobahnPython/blob/issue208/autobahn/autobahn/twisted/wamp.py#L254 and the docs to https://github.com/tavendo/AutobahnPython/blob/issue208/doc/wamp2app.rst

robertlagrant commented 10 years ago

I'm a complete outsider with a blog post-reading level of interest in crossbar.io (at the moment) but this issue's discussion and direction are brilliant. Convinced me to keep watching.

DenJohX commented 9 years ago

It would be great if Application use a reconnecting factory, just like autobahn.js

oberstet commented 9 years ago

@DenJohX I agree https://github.com/tavendo/AutobahnPython/issues/295

petercsmith commented 9 years ago

@DenJohX Agreed, not being able to automatically reconnect a python client (using ApplicationRunner) to crossbar.io is blocking me from using Autobahn. Is there a workaround?

oberstet commented 6 years ago

subseded/fixed in https://github.com/crossbario/autobahn-python/issues/964