Divij/client message flow #51
Conversation
| raise NotImplementedError | ||
|
|
||
| def on_join(self, *args, **kwargs): |
There was a problem hiding this comment.
Shouldn't onjoin take the sockets as an argument.
There was a problem hiding this comment.
This is a basic method that accepts absolutely anything via *args and **kwargs. Ideally, any application that needs access to the socket can specify its the arguments in its own on_join method.
…ce. Minor style and spelling fixes.
| cls._instantiate_for_interview(interview_id) | ||
| cls._apps_instantiated = True | ||
|
|
||
| cls._setup_apps(interview_id, client_socket) |
There was a problem hiding this comment.
setup_apps seems like a strange name for this since it just notifies all apps that someone has joined the interview. I'd probably call it something like notifiy_apps_client_joined or something like that
There was a problem hiding this comment.
That's more descriptive. Will do.
| _apps_instantiated = False | ||
|
|
||
| @classmethod | ||
| def initialize(cls, interview_id, client_socket): |
There was a problem hiding this comment.
Can we switch this to self from cls?
There was a problem hiding this comment.
Per conversation with Jesse nope.
There was a problem hiding this comment.
cls and self have different meanings in Python. self is used to reference instance variables and cls class variables. When using the @classmethod decorator, the first argument to the method must always be cls and not self
There was a problem hiding this comment.
By the way I'm going to put up a PR today that will get this in. —
Sent from Mailbox
On Sat, Jun 7, 2014 at 5:21 PM, Divij Rajkumar notifications@github.com
wrote:
+from akobi.lib.utils import function_as_callback
+
+
+class Initializer(object):
- """
- Initialize the interview for a particular client. Called by the
- WebSocketHandler's on_message method when it receives an init
- message from the client (which happens whenever a new client joins)
- """
This class is never going to be instantiated, so having a Borg here
isn't going to work. Instead just use a class variable
- _apps_instantiated = False
- @classmethod
- def initialize(cls, interview_id, client_socket):
clsandselfhave different meanings in Python.selfis used to reference instance variables andclsclass variables. When using the@classmethoddecorator, the first argument to the method must always beclsand notself
Reply to this email directly or view it on GitHub:
https://github.com/jivid/akobi/pull/51/files#r13521501
|
shaep it |
|
Lick it |
|
shipwrecked! |
|
Not quiet ready. Working on a non type error that's appeared. |
…l apps instantiated boolean.
|
Ship it! |
| # everytime a client connects, we move this logic into the registry | ||
| # so the initializer can be left stateless | ||
| if self.interviews[interview_id][app_name] is not None: | ||
| log.info("%s has already been instantiated for %s" % (app_name, |
There was a problem hiding this comment.
Probably don't need this log, definately not at an info level.
|
fix n ship |
Wiring up application registration with client "init_interview" message.
For the time being we are hardcoding registered applications see line 52, 53 on interview.py. Registration via selection screen to follow.
PEP8 Compliant. Tested notes and heartbeat application.