From f05022d39b530069a75fbecfd744bca66cc0ce66 Mon Sep 17 00:00:00 2001 From: Sam Thursfield Date: Thu, 10 Apr 2014 08:08:40 +0000 Subject: distbuild: Consistently name all maybe_ message callbacks There are cases where a state machine handles an event but stays in the same state. A callback is registered which filters messages further before possibly taking action. There have been bugs caused by this pattern being incorrectly implemented (where the callback is expected to filter the message, but a transition takes place anyway). Hopefully a consistent naming convention will make the pattern clearer. --- distbuild/build_controller.py | 46 ++++++++++++++++++------------------- distbuild/worker_build_scheduler.py | 4 ++-- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/distbuild/build_controller.py b/distbuild/build_controller.py index d750286d..404bcf9f 100644 --- a/distbuild/build_controller.py +++ b/distbuild/build_controller.py @@ -175,9 +175,9 @@ class BuildController(distbuild.StateMachine): distbuild.InitiatorDisconnect, None, None), ('graphing', distbuild.HelperRouter, distbuild.HelperOutput, - 'graphing', self._collect_graph), + 'graphing', self._maybe_collect_graph), ('graphing', distbuild.HelperRouter, distbuild.HelperResult, - 'graphing', self._finish_graph), + 'graphing', self._maybe_finish_graph), ('graphing', self, _GotGraph, 'annotating', self._start_annotating), ('graphing', self, _GraphFailed, None, None), @@ -185,8 +185,8 @@ class BuildController(distbuild.StateMachine): distbuild.InitiatorDisconnect, None, None), ('annotating', distbuild.HelperRouter, distbuild.HelperResult, - 'annotating', self._handle_cache_response), - ('annotating', self, _Annotated, 'building', + 'annotating', self._maybe_handle_cache_response), + ('annotating', self, _Annotated, 'building', self._queue_worker_builds), ('annotating', self._initiator_connection, distbuild.InitiatorDisconnect, None, None), @@ -198,18 +198,18 @@ class BuildController(distbuild.StateMachine): # specific to WorkerConnection instances that our currently # building for us, but the state machines are not intended to # behave that way). - ('building', distbuild.WorkerConnection, - distbuild.WorkerBuildStepStarted, 'building', - self._relay_build_step_started), - ('building', distbuild.WorkerConnection, - distbuild.WorkerBuildOutput, 'building', - self._relay_build_output), - ('building', distbuild.WorkerConnection, - distbuild.WorkerBuildCaching, 'building', - self._relay_build_caching), - ('building', distbuild.WorkerConnection, - distbuild.WorkerBuildFinished, 'building', - self._check_result_and_queue_more_builds), + ('building', distbuild.WorkerConnection, + distbuild.WorkerBuildStepStarted, 'building', + self._maybe_relay_build_step_started), + ('building', distbuild.WorkerConnection, + distbuild.WorkerBuildOutput, 'building', + self._maybe_relay_build_output), + ('building', distbuild.WorkerConnection, + distbuild.WorkerBuildCaching, 'building', + self._maybe_relay_build_caching), + ('building', distbuild.WorkerConnection, + distbuild.WorkerBuildFinished, 'building', + self._maybe_check_result_and_queue_more_builds), ('building', distbuild.WorkerConnection, distbuild.WorkerBuildFailed, 'building', self._maybe_notify_build_failed), @@ -248,14 +248,14 @@ class BuildController(distbuild.StateMachine): progress = BuildProgress(self._request['id'], 'Computing build graph') self.mainloop.queue_event(BuildController, progress) - def _collect_graph(self, event_source, event): + def _maybe_collect_graph(self, event_source, event): distbuild.crash_point() if event.msg['id'] == self._helper_id: self._artifact_data.add(event.msg['stdout']) self._artifact_error.add(event.msg['stderr']) - def _finish_graph(self, event_source, event): + def _maybe_finish_graph(self, event_source, event): distbuild.crash_point() def notify_failure(msg_text): @@ -328,7 +328,7 @@ class BuildController(distbuild.StateMachine): 'Queued as %s query whether %s is in cache' % (msg['id'], filename)) - def _handle_cache_response(self, event_source, event): + def _maybe_handle_cache_response(self, event_source, event): distbuild.crash_point() logging.debug('Got cache query response: %s' % repr(event.msg)) @@ -426,7 +426,7 @@ class BuildController(distbuild.StateMachine): cancel = BuildCancel(disconnect.id) self.mainloop.queue_event(BuildController, cancel) - def _relay_build_step_started(self, event_source, event): + def _maybe_relay_build_step_started(self, event_source, event): distbuild.crash_point() if event.initiator_id != self._request['id']: return # not for us @@ -444,7 +444,7 @@ class BuildController(distbuild.StateMachine): self.mainloop.queue_event(BuildController, started) logging.debug('BC: emitted %s' % repr(started)) - def _relay_build_output(self, event_source, event): + def _maybe_relay_build_output(self, event_source, event): distbuild.crash_point() if event.msg['id'] != self._request['id']: return # not for us @@ -462,7 +462,7 @@ class BuildController(distbuild.StateMachine): self.mainloop.queue_event(BuildController, output) logging.debug('BC: queued %s' % repr(output)) - def _relay_build_caching(self, event_source, event): + def _maybe_relay_build_caching(self, event_source, event): distbuild.crash_point() if event.initiator_id != self._request['id']: return # not for us @@ -485,7 +485,7 @@ class BuildController(distbuild.StateMachine): else: return None - def _check_result_and_queue_more_builds(self, event_source, event): + def _maybe_check_result_and_queue_more_builds(self, event_source, event): distbuild.crash_point() if event.msg['id'] != self._request['id']: return # not for us diff --git a/distbuild/worker_build_scheduler.py b/distbuild/worker_build_scheduler.py index 9ada07ef..f17a4cf4 100644 --- a/distbuild/worker_build_scheduler.py +++ b/distbuild/worker_build_scheduler.py @@ -226,7 +226,7 @@ class WorkerConnection(distbuild.StateMachine): self._request_caching), ('caching', distbuild.HelperRouter, distbuild.HelperResult, - 'caching', self._handle_helper_result), + 'caching', self._maybe_handle_helper_result), ('caching', self, _Cached, 'idle', self._request_job), ('caching', self, _JobFailed, 'idle', self._request_job), ] @@ -374,7 +374,7 @@ class WorkerConnection(distbuild.StateMachine): self._initiator_id = None self._finished_msg = event.msg - def _handle_helper_result(self, event_source, event): + def _maybe_handle_helper_result(self, event_source, event): if event.msg['id'] == self._helper_id: distbuild.crash_point() -- cgit v1.2.1