Hi Cristobal, just took a peek at Erebus and looks good! There's some low hanging fruit for improvement (unused functions and such) but generally looks good. Cheers! -Damian ====================================================================== erebus/server/handlers/log.py ====================================================================== 9 import erebus.util.log Perfectly fine for now, but obviously most of this is copy/paste from Nyx. Both Nyx and Erebus are smack in the middle of development so this is great! Do whatever you find most expedient. But obviously in the long run we'll want to revisit this. ---------------------------------------------------------------------- 16 def conf_handler(key, value): 17 if key == 'features.log.maxLinesPerEntry': 18 return max(1, value) 19 elif key == 'features.log.prepopulateReadLimit': 20 return max(0, value) 21 elif key == 'features.log.maxRefreshRate': 22 return max(10, value) 23 elif key == 'cache.log_panel.size': 24 return max(1000, value) I know this is copy-and-paste from Nyx but personally I wouldn't bother. Many of the config keys are unused, and at present users don't have a way of customizing them. Nyx's support for a nyxrc is a very rarely used feature, so I would leave this out for now. ---------------------------------------------------------------------- 40 TOR_EVENT_TYPES = { 41 'd': 'DEBUG', 42 'i': 'INFO', 43 'n': 'NOTICE', 44 'w': 'WARN', 45 'e': 'ERR', I did this for Nyx because because... well, curses. For Erebus we can do better. ;) Rather than 'key => event' mapping maybe Erebus should show all the event types, with checkboxes for selection? ---------------------------------------------------------------------- 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. The 'output' is only ever used on this last line, so might as well just call... ws.send_data(WebSocketType.LOG, {'message': event.display_message, 'type': event.type.lower()}) ---------------------------------------------------------------------- 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. ---------------------------------------------------------------------- 195 response = None 196 if controller is not None: 197 response = controller.get_info('events/names', None) 198 if response is None: 199 return [] # GETINFO query failed This is equivalent to... response = controller.get_info('events/names', []) if controller else [] ====================================================================== erebus/util/controller.py ====================================================================== 29 def erebus_controller(): 30 """ 31 Provides the erebus controller instance. 32 """ 33 34 return EREBUS_CONTROLLER 35 36 def init_controller(control_port, control_socket, logged_events): 37 """ 38 Spawns the controller 39 """ 40 41 global EREBUS_CONTROLLER 42 EREBUS_CONTROLLER = Controller(control_port, control_socket, logged_events) Again, you call init_controller() but looks as though EREBUS_CONTROLLER is never used. ---------------------------------------------------------------------- 61 self._logged_events = logged_events 62 63 self._bw_handler = None 64 self._status_handler = None 65 self._log_handler = None All unused. ---------------------------------------------------------------------- 86 def _stop_loop_check(self): 87 """ 88 Stop the looping task when is not necessary 89 """ 90 91 self._loop_call.stop() Only called once, in _conn_starter(). Might as well just call this there (it's just a minor alias). ---------------------------------------------------------------------- 156 def heartbeat_check(is_unresponsive): 157 """ 158 Logs if its been ten seconds since the last BW event. 159 160 Arguments: 161 is_unresponsive - flag for if we've indicated to be responsive or not 162 """ 163 164 controller = tor_controller() 165 last_heartbeat = controller.get_latest_heartbeat() 166 167 if controller.is_alive(): 168 if not is_unresponsive and (time.time() - last_heartbeat) >= 10: 169 is_unresponsive = True 170 log.notice('Relay unresponsive (last heartbeat: %s)' % time.ctime(last_heartbeat)) 171 elif is_unresponsive and (time.time() - last_heartbeat) < 10: 172 # really shouldn't happen (meant Tor froze for a bit) 173 is_unresponsive = False 174 log.notice('Relay resumed') 175 176 return is_unresponsive Unused. ====================================================================== erebus/util/websockets.py ====================================================================== 41 def add_websocket(self, ws_type, ws): 42 """ 43 Add an object to the list of active websockets of certain type. 44 45 Arguments: 46 ws_type - type of websocket to be fetched 47 ws - websocket object to be added 48 """ 49 50 if self._websockets[ws_type] is not None: 51 if ws not in self._websockets[ws_type]: 52 self._websockets[ws_type].append(ws) 53 else: 54 log.notice('Undefined type (%s) when adding websocket' % ws_type) Your 'if self._websockets[ws_type] is not None' check will never evaluate to False. You default all WebSocketType values to an empty list, and if it doesn't belong to that enum then it'll raise a KeyError. You want... if ws_type in self._websockets: Also, if self._websockets had sets rather than lists then you could drop the second check. 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.