[tor-dev] Erebus Code Review

[Cristobal]

Hi Damian, thanks for your feedback! I'm replying a few things inline.


On 05/09/15 17:02, Damian Johnson wrote:
> ----------------------------------------------------------------------
> 
> 155     def _register_event(self, event):
> 156         output = { 'message': '', 'type': ''}
> 157         if event.type not in self._logged_events:
> 158             return
> 159         #self._event_log.add(event)
> 160         #self._log_file.write(event.display_message)
> 161
> 162         # notifies the display that it has new content
> 163         if self._filter.match(event.display_message):
> 164             self._has_new_event = True
> 165             output['message'] = event.display_message
> 166             output['type'] = event.type.lower()
> 167             ws = ws_controller()
> 168             if ws is not None:
> 169                 ws.send_data(WebSocketType.LOG, output)
> 
> You can drop the line with self._has_new_event. It's an unused
> attribute, and doesn't appear outside of this line.

I'm still keeping a few nyx lines around because I'm trying to do the
following:

* When erebus starts, it will send log entries from server to client
dashboard. Which logs to send will depend on how erebus was invoked
(e.g. run_erebus -L A for logging all event types).
* The user would be able to change logging event types from the client
dashboard (on a checkbox), but this selection has to be reflected on the
server too, otherwise the client dashboard might be filtering certain
type of events just on the frontend but the server will still be sending
other type of events.
* My goal is to make the client dashboard to send a notification to the
server informing which new event types to log, and the server will
update its behavior. The client dashboard will then continue to receive
log event types without doing any changes by itself, because the server
would be the one who updated its log event filter.

Something similar happens with log dedup, and {log,bw} prepopulation.

I'm very close to make this work, by defining a kind of "protocol" in
which the client dashboard can send notifications to the server. If this
works, it could be the starting point to further add functionalities as
"editing the relay nickname" (and such) from the client dashboard.

> 171 def log_handler():
> 172     """
> 173     Singleton for getting our log hander.
> 174
> 175     :returns: :class:`~erebus.server.handlers.log.LogHandler`
> 176     """
> 177
> 178     return LOG_HANDLER
> 179
> 180 def init_log_handler(logged_events):
> 181     """
> 182     """
> 183     global LOG_HANDLER
> 184     LOG_HANDLER = LogHandler(logged_events)
> 185     return LOG_HANDLER
> 
> Seems from a quick look that these might be unused. The controller.py
> and starter.py call init_log_handler, but not spotting anything that
> ever uses LOG_HANDLER.

I first call init_log_handler on starter.py. Then, when the Tor control
connection is made, I call log_handler again (on
controller.py:_start_tor_controller).

> 
> Btw, lets use Sphinx pydocs. The documentation style you're using
> here is adopted from Nyx before I started using Sphinx, and I'm
> replacing that documentation style as I rewrite it.
> 

Will do.

-
Cristobal

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.torproject.org/pipermail/tor-dev/attachments/20150909/449b559e/attachment-0001.sig>