)]}'
{"oslo_messaging/_drivers/protocols/amqp/controller.py":[{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"47f19274274c1d6771b0b6dbbae79ac55c308845","unresolved":false,"context_lines":[{"line_number":498,"context_line":"               count \u003c self._max_task_batch and"},{"line_number":499,"context_line":"               self._can_process_tasks):"},{"line_number":500,"context_line":"            try:"},{"line_number":501,"context_line":"                task \u003d self._tasks.get(False)"},{"line_number":502,"context_line":"                task.execute(self)"},{"line_number":503,"context_line":"            except Exception as e:"},{"line_number":504,"context_line":"                LOG.exception(\"Error processing task: %s\", e)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_05428268","line":501,"updated":"2015-10-15 15:32:56.000000000","message":"unrelated change?","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"78db45db01f1c19b0d85bddd4d650e63dc258f39","unresolved":false,"context_lines":[{"line_number":498,"context_line":"               count \u003c self._max_task_batch and"},{"line_number":499,"context_line":"               self._can_process_tasks):"},{"line_number":500,"context_line":"            try:"},{"line_number":501,"context_line":"                task \u003d self._tasks.get(False)"},{"line_number":502,"context_line":"                task.execute(self)"},{"line_number":503,"context_line":"            except Exception as e:"},{"line_number":504,"context_line":"                LOG.exception(\"Error processing task: %s\", e)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_fdefbb84","line":501,"in_reply_to":"5a2ca52d_05428268","updated":"2015-10-16 09:30:20.000000000","message":"Yup, debugging what turned out to be an unrelated problem and accidentally committed.","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"ca1faf04a584c00f655144bc8cfdd574fa37b9df","unresolved":false,"context_lines":[{"line_number":498,"context_line":"               count \u003c self._max_task_batch and"},{"line_number":499,"context_line":"               self._can_process_tasks):"},{"line_number":500,"context_line":"            try:"},{"line_number":501,"context_line":"                task \u003d self._tasks.get(False)"},{"line_number":502,"context_line":"                task.execute(self)"},{"line_number":503,"context_line":"            except Exception as e:"},{"line_number":504,"context_line":"                LOG.exception(\"Error processing task: %s\", e)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_3efa0944","line":501,"in_reply_to":"5a2ca52d_fdefbb84","updated":"2015-10-16 09:57:44.000000000","message":"yes, lets drop this please","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"47f19274274c1d6771b0b6dbbae79ac55c308845","unresolved":false,"context_lines":[{"line_number":518,"context_line":"                already_scheduled \u003d self._process_tasks_scheduled"},{"line_number":519,"context_line":"                self._process_tasks_scheduled \u003d True"},{"line_number":520,"context_line":"            if not already_scheduled:"},{"line_number":521,"context_line":"                self.processor.wakeup(self._process_tasks)"},{"line_number":522,"context_line":""},{"line_number":523,"context_line":"    @property"},{"line_number":524,"context_line":"    def _can_process_tasks(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_c5600a04","line":521,"updated":"2015-10-15 15:32:56.000000000","message":"unrelated change?","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"ca1faf04a584c00f655144bc8cfdd574fa37b9df","unresolved":false,"context_lines":[{"line_number":518,"context_line":"                already_scheduled \u003d self._process_tasks_scheduled"},{"line_number":519,"context_line":"                self._process_tasks_scheduled \u003d True"},{"line_number":520,"context_line":"            if not already_scheduled:"},{"line_number":521,"context_line":"                self.processor.wakeup(self._process_tasks)"},{"line_number":522,"context_line":""},{"line_number":523,"context_line":"    @property"},{"line_number":524,"context_line":"    def _can_process_tasks(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_fe96d1ee","line":521,"in_reply_to":"5a2ca52d_bde5c3a4","updated":"2015-10-16 09:57:44.000000000","message":"yes, let\u0027s drop this please","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"78db45db01f1c19b0d85bddd4d650e63dc258f39","unresolved":false,"context_lines":[{"line_number":518,"context_line":"                already_scheduled \u003d self._process_tasks_scheduled"},{"line_number":519,"context_line":"                self._process_tasks_scheduled \u003d True"},{"line_number":520,"context_line":"            if not already_scheduled:"},{"line_number":521,"context_line":"                self.processor.wakeup(self._process_tasks)"},{"line_number":522,"context_line":""},{"line_number":523,"context_line":"    @property"},{"line_number":524,"context_line":"    def _can_process_tasks(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_bde5c3a4","line":521,"in_reply_to":"5a2ca52d_c5600a04","updated":"2015-10-16 09:30:20.000000000","message":"Ditto, although an improvement anyway :) Will remove.","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"}],"oslo_messaging/server.py":[{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"47f19274274c1d6771b0b6dbbae79ac55c308845","unresolved":false,"context_lines":[{"line_number":39,"context_line":""},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"# The current state of the executor is defined by 2 pieces of data:"},{"line_number":42,"context_line":"#   self._executor: the executor object itself"},{"line_number":43,"context_line":"#   self._executor_state: the state of the executor object"},{"line_number":44,"context_line":"#"},{"line_number":45,"context_line":"# The reason it is not just _executor_state is that the state \u0027wraps\u0027 when"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_6838ebef","line":42,"updated":"2015-10-15 15:32:56.000000000","message":"There are references to self.executor below as well as self._executor - isn\u0027t this a problem?","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"4c1153f596ca51ed0977efc69ce8cf81a7c90a3a","unresolved":false,"context_lines":[{"line_number":39,"context_line":""},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"# The current state of the executor is defined by 2 pieces of data:"},{"line_number":42,"context_line":"#   self._executor: the executor object itself"},{"line_number":43,"context_line":"#   self._executor_state: the state of the executor object"},{"line_number":44,"context_line":"#"},{"line_number":45,"context_line":"# The reason it is not just _executor_state is that the state \u0027wraps\u0027 when"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_ae728b0f","line":42,"in_reply_to":"5a2ca52d_6838ebef","updated":"2015-10-15 15:56:48.000000000","message":"Ah never mind self.executor is the type of executor (string)","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"78db45db01f1c19b0d85bddd4d650e63dc258f39","unresolved":false,"context_lines":[{"line_number":39,"context_line":""},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"# The current state of the executor is defined by 2 pieces of data:"},{"line_number":42,"context_line":"#   self._executor: the executor object itself"},{"line_number":43,"context_line":"#   self._executor_state: the state of the executor object"},{"line_number":44,"context_line":"#"},{"line_number":45,"context_line":"# The reason it is not just _executor_state is that the state \u0027wraps\u0027 when"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_ddb8bf77","line":42,"in_reply_to":"5a2ca52d_ae728b0f","updated":"2015-10-16 09:30:20.000000000","message":"Yeah, I also found that confusing. I wasn\u0027t going to change it, but I can if you like.","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"ca1faf04a584c00f655144bc8cfdd574fa37b9df","unresolved":false,"context_lines":[{"line_number":39,"context_line":""},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"# The current state of the executor is defined by 2 pieces of data:"},{"line_number":42,"context_line":"#   self._executor: the executor object itself"},{"line_number":43,"context_line":"#   self._executor_state: the state of the executor object"},{"line_number":44,"context_line":"#"},{"line_number":45,"context_line":"# The reason it is not just _executor_state is that the state \u0027wraps\u0027 when"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_9e88fdc6","line":42,"in_reply_to":"5a2ca52d_ddb8bf77","updated":"2015-10-16 09:57:44.000000000","message":"yes please i think its related to make this change better","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"9ba3b3ef121eadc97c7d06f05ad916e525d09285","unresolved":false,"context_lines":[{"line_number":60,"context_line":"#"},{"line_number":61,"context_line":"# _executor_state proceeds strictly through the following sequence. Note that"},{"line_number":62,"context_line":"# we compare these states with \u003c and \u003e (any state before X, any state after X),"},{"line_number":63,"context_line":"# so their values must be ordered."},{"line_number":64,"context_line":"EXECUTOR_UNINITIALIZED \u003d 0"},{"line_number":65,"context_line":"EXECUTOR_INITIALIZING \u003d 1"},{"line_number":66,"context_line":"EXECUTOR_RUNNING \u003d 2"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_99535be2","line":63,"updated":"2015-10-15 16:06:10.000000000","message":"Do u want these exposed to end-users (perhaps if not prefix them with \"_\")","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"ca1faf04a584c00f655144bc8cfdd574fa37b9df","unresolved":false,"context_lines":[{"line_number":60,"context_line":"#"},{"line_number":61,"context_line":"# _executor_state proceeds strictly through the following sequence. Note that"},{"line_number":62,"context_line":"# we compare these states with \u003c and \u003e (any state before X, any state after X),"},{"line_number":63,"context_line":"# so their values must be ordered."},{"line_number":64,"context_line":"EXECUTOR_UNINITIALIZED \u003d 0"},{"line_number":65,"context_line":"EXECUTOR_INITIALIZING \u003d 1"},{"line_number":66,"context_line":"EXECUTOR_RUNNING \u003d 2"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_1ec38dee","line":63,"in_reply_to":"5a2ca52d_5da58f44","updated":"2015-10-16 09:57:44.000000000","message":"ok we can do this later","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"78db45db01f1c19b0d85bddd4d650e63dc258f39","unresolved":false,"context_lines":[{"line_number":60,"context_line":"#"},{"line_number":61,"context_line":"# _executor_state proceeds strictly through the following sequence. Note that"},{"line_number":62,"context_line":"# we compare these states with \u003c and \u003e (any state before X, any state after X),"},{"line_number":63,"context_line":"# so their values must be ordered."},{"line_number":64,"context_line":"EXECUTOR_UNINITIALIZED \u003d 0"},{"line_number":65,"context_line":"EXECUTOR_INITIALIZING \u003d 1"},{"line_number":66,"context_line":"EXECUTOR_RUNNING \u003d 2"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_5da58f44","line":63,"in_reply_to":"5a2ca52d_99535be2","updated":"2015-10-16 09:30:20.000000000","message":"They\u0027re an implementation detail which we might reasonably change (for example, if we later discovered that the STARTING state is required after all). Nothing similar is currently exposed. The caller should already have this context, anyway.","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"0268bb6407d5e52d859c3430388838b3fe3ab5b2","unresolved":false,"context_lines":[{"line_number":61,"context_line":"# _executor_state proceeds strictly through the following sequence. Note that"},{"line_number":62,"context_line":"# we compare these states with \u003c and \u003e (any state before X, any state after X),"},{"line_number":63,"context_line":"# so their values must be ordered."},{"line_number":64,"context_line":"EXECUTOR_UNINITIALIZED \u003d 0"},{"line_number":65,"context_line":"EXECUTOR_INITIALIZING \u003d 1"},{"line_number":66,"context_line":"EXECUTOR_RUNNING \u003d 2"},{"line_number":67,"context_line":"EXECUTOR_STOPPING \u003d 3"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_eec63e99","line":64,"updated":"2015-10-15 17:31:57.000000000","message":"Other idea, use a transition table,\n\nSimilar to https://github.com/openstack/taskflow/blob/master/taskflow/states.py#L89\n\nOr use https://pypi.python.org/pypi/enum34 which has a nice IntEnum...\n\nOr use automaton to help, but that might be overkill for this right now...","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"ca1faf04a584c00f655144bc8cfdd574fa37b9df","unresolved":false,"context_lines":[{"line_number":61,"context_line":"# _executor_state proceeds strictly through the following sequence. Note that"},{"line_number":62,"context_line":"# we compare these states with \u003c and \u003e (any state before X, any state after X),"},{"line_number":63,"context_line":"# so their values must be ordered."},{"line_number":64,"context_line":"EXECUTOR_UNINITIALIZED \u003d 0"},{"line_number":65,"context_line":"EXECUTOR_INITIALIZING \u003d 1"},{"line_number":66,"context_line":"EXECUTOR_RUNNING \u003d 2"},{"line_number":67,"context_line":"EXECUTOR_STOPPING \u003d 3"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_9eaf9d29","line":64,"in_reply_to":"5a2ca52d_5d2a6f91","updated":"2015-10-16 09:57:44.000000000","message":"ok we can do this later","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"78db45db01f1c19b0d85bddd4d650e63dc258f39","unresolved":false,"context_lines":[{"line_number":61,"context_line":"# _executor_state proceeds strictly through the following sequence. Note that"},{"line_number":62,"context_line":"# we compare these states with \u003c and \u003e (any state before X, any state after X),"},{"line_number":63,"context_line":"# so their values must be ordered."},{"line_number":64,"context_line":"EXECUTOR_UNINITIALIZED \u003d 0"},{"line_number":65,"context_line":"EXECUTOR_INITIALIZING \u003d 1"},{"line_number":66,"context_line":"EXECUTOR_RUNNING \u003d 2"},{"line_number":67,"context_line":"EXECUTOR_STOPPING \u003d 3"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_5d2a6f91","line":64,"in_reply_to":"5a2ca52d_eec63e99","updated":"2015-10-16 09:30:20.000000000","message":"The enum would be nice, didn\u0027t realise there was a backport. Will look at that. The problem with a regular state machine is that you can\u0027t typically compare 2 arbitrary states and say X strictly comes after Y, transitively. It is convenient in this case to be able to do that.","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"0268bb6407d5e52d859c3430388838b3fe3ab5b2","unresolved":false,"context_lines":[{"line_number":174,"context_line":""},{"line_number":175,"context_line":"        # NOTE: We\u0027ve got a race here. If another thread sends us a message"},{"line_number":176,"context_line":"        # before initialisation is complete, but without changing the blocking"},{"line_number":177,"context_line":"        # executor itself there\u0027s obviously nothing we can do about that here."},{"line_number":178,"context_line":"        if self.executor \u003d\u003d \u0027blocking\u0027:"},{"line_number":179,"context_line":"            with self._state_cond:"},{"line_number":180,"context_line":"                self._executor \u003d executor"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_0e95ba3d","line":177,"updated":"2015-10-15 17:31:57.000000000","message":"Would doing the following fix that:\n\n  self._executor_state \u003d EXECUTOR_STARTING\n  executor.start()\n  self._executor_state \u003d EXECUTOR_RUNNING\n  self._executor \u003d executor\n\nDon\u0027t allow messages to be sent until \u0027EXECUTOR_RUNNING\u0027  and self._executor is not None?","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"78db45db01f1c19b0d85bddd4d650e63dc258f39","unresolved":false,"context_lines":[{"line_number":174,"context_line":""},{"line_number":175,"context_line":"        # NOTE: We\u0027ve got a race here. If another thread sends us a message"},{"line_number":176,"context_line":"        # before initialisation is complete, but without changing the blocking"},{"line_number":177,"context_line":"        # executor itself there\u0027s obviously nothing we can do about that here."},{"line_number":178,"context_line":"        if self.executor \u003d\u003d \u0027blocking\u0027:"},{"line_number":179,"context_line":"            with self._state_cond:"},{"line_number":180,"context_line":"                self._executor \u003d executor"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_d811ad05","line":177,"in_reply_to":"5a2ca52d_0e95ba3d","updated":"2015-10-16 09:30:20.000000000","message":"I\u0027m going to remove the race comment here, because on looking harder I don\u0027t think there\u0027s a race. Please read my assumption carefully, though, because I have not read all the code! I am assuming that it is safe for a message to be dispatched at this point because the listener already exists and will simply queue messages. Messages will eventually be processed in order when the executor actually starts executing.","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"ca1faf04a584c00f655144bc8cfdd574fa37b9df","unresolved":false,"context_lines":[{"line_number":174,"context_line":""},{"line_number":175,"context_line":"        # NOTE: We\u0027ve got a race here. If another thread sends us a message"},{"line_number":176,"context_line":"        # before initialisation is complete, but without changing the blocking"},{"line_number":177,"context_line":"        # executor itself there\u0027s obviously nothing we can do about that here."},{"line_number":178,"context_line":"        if self.executor \u003d\u003d \u0027blocking\u0027:"},{"line_number":179,"context_line":"            with self._state_cond:"},{"line_number":180,"context_line":"                self._executor \u003d executor"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_9e76bdd5","line":177,"in_reply_to":"5a2ca52d_d811ad05","updated":"2015-10-16 09:57:44.000000000","message":"sounds good.","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"bdc1c216a9aedd9e9bfb8b7417310a43160e29b2","unresolved":false,"context_lines":[{"line_number":183,"context_line":""},{"line_number":184,"context_line":"            executor.start()"},{"line_number":185,"context_line":""},{"line_number":186,"context_line":"        else:"},{"line_number":187,"context_line":"            with self._state_cond:"},{"line_number":188,"context_line":"                # N.B. We assume that this is ok, because it is not blocking"},{"line_number":189,"context_line":"                executor.start()"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_59eb8313","line":186,"updated":"2015-10-15 16:10:50.000000000","message":"Do we need the check at all? can we always set \"executor_state \u003d EXECUTOR_RUNNING\", then call executor.start() for both cases?","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"0268bb6407d5e52d859c3430388838b3fe3ab5b2","unresolved":false,"context_lines":[{"line_number":183,"context_line":""},{"line_number":184,"context_line":"            executor.start()"},{"line_number":185,"context_line":""},{"line_number":186,"context_line":"        else:"},{"line_number":187,"context_line":"            with self._state_cond:"},{"line_number":188,"context_line":"                # N.B. We assume that this is ok, because it is not blocking"},{"line_number":189,"context_line":"                executor.start()"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_cea8227a","line":186,"in_reply_to":"5a2ca52d_59eb8313","updated":"2015-10-15 17:31:57.000000000","message":"Or have a new \u0027EXECUTOR_STARTING\u0027 which after changed starts the executor, then after that works change state to \u0027EXECUTOR_RUNNING\u0027, don\u0027t let message(s) be sent until \u0027EXECUTOR_RUNNING\u0027 ?","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"ca1faf04a584c00f655144bc8cfdd574fa37b9df","unresolved":false,"context_lines":[{"line_number":183,"context_line":""},{"line_number":184,"context_line":"            executor.start()"},{"line_number":185,"context_line":""},{"line_number":186,"context_line":"        else:"},{"line_number":187,"context_line":"            with self._state_cond:"},{"line_number":188,"context_line":"                # N.B. We assume that this is ok, because it is not blocking"},{"line_number":189,"context_line":"                executor.start()"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_de46757a","line":186,"in_reply_to":"5a2ca52d_c309be85","updated":"2015-10-16 09:57:44.000000000","message":"sounds good","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"78db45db01f1c19b0d85bddd4d650e63dc258f39","unresolved":false,"context_lines":[{"line_number":183,"context_line":""},{"line_number":184,"context_line":"            executor.start()"},{"line_number":185,"context_line":""},{"line_number":186,"context_line":"        else:"},{"line_number":187,"context_line":"            with self._state_cond:"},{"line_number":188,"context_line":"                # N.B. We assume that this is ok, because it is not blocking"},{"line_number":189,"context_line":"                executor.start()"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_c309be85","line":186,"in_reply_to":"5a2ca52d_cea8227a","updated":"2015-10-16 09:30:20.000000000","message":"See above.","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"bdc1c216a9aedd9e9bfb8b7417310a43160e29b2","unresolved":false,"context_lines":[{"line_number":203,"context_line":"        with self._state_cond:"},{"line_number":204,"context_line":"            # We must not call executor.stop() before executor.start() has"},{"line_number":205,"context_line":"            # completed. Wait for that here."},{"line_number":206,"context_line":"            executor \u003d self._executor"},{"line_number":207,"context_line":"            while (self._executor \u003d\u003d executor and"},{"line_number":208,"context_line":"                    self._executor_state \u003c EXECUTOR_RUNNING):"},{"line_number":209,"context_line":"                # Executor is still initializing"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_99bcbb07","line":206,"updated":"2015-10-15 16:10:50.000000000","message":"can we call this current_executor? since we already have self.executor and self._executor","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"42aedb2c35d290b5383dd31d615b0b790556b5b4","unresolved":false,"context_lines":[{"line_number":203,"context_line":"        with self._state_cond:"},{"line_number":204,"context_line":"            # We must not call executor.stop() before executor.start() has"},{"line_number":205,"context_line":"            # completed. Wait for that here."},{"line_number":206,"context_line":"            executor \u003d self._executor"},{"line_number":207,"context_line":"            while (self._executor \u003d\u003d executor and"},{"line_number":208,"context_line":"                    self._executor_state \u003c EXECUTOR_RUNNING):"},{"line_number":209,"context_line":"                # Executor is still initializing"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_a07905ca","line":206,"in_reply_to":"5a2ca52d_9801f53a","updated":"2015-10-16 15:36:13.000000000","message":"Agreed, not your fault at all, just a weirdness that we can squash now vs having to continue to support it; and squashing it makes things simpler, which I\u0027m a fan of :-P We might want to tell people about this change though, if someone is doing stop, start, stop, start, ... for some reason, this will now break (which I think is ok, but they might not think it is ok...)","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"ea59860695bb93e9ed778582e4da56a45fe9cd14","unresolved":false,"context_lines":[{"line_number":203,"context_line":"        with self._state_cond:"},{"line_number":204,"context_line":"            # We must not call executor.stop() before executor.start() has"},{"line_number":205,"context_line":"            # completed. Wait for that here."},{"line_number":206,"context_line":"            executor \u003d self._executor"},{"line_number":207,"context_line":"            while (self._executor \u003d\u003d executor and"},{"line_number":208,"context_line":"                    self._executor_state \u003c EXECUTOR_RUNNING):"},{"line_number":209,"context_line":"                # Executor is still initializing"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_12c62168","line":206,"in_reply_to":"5a2ca52d_9801f53a","updated":"2015-10-16 10:12:09.000000000","message":"It\u0027s test_two_pools_three_listener. It hangs on:\n  self.wait_for_messages(75, \u0027pool2\u0027)\nbecause listener2_thread was stopped and then started above. I\u0027m reading the test in a bit more detail to try to work out why it\u0027s doing this.","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"ca1faf04a584c00f655144bc8cfdd574fa37b9df","unresolved":false,"context_lines":[{"line_number":203,"context_line":"        with self._state_cond:"},{"line_number":204,"context_line":"            # We must not call executor.stop() before executor.start() has"},{"line_number":205,"context_line":"            # completed. Wait for that here."},{"line_number":206,"context_line":"            executor \u003d self._executor"},{"line_number":207,"context_line":"            while (self._executor \u003d\u003d executor and"},{"line_number":208,"context_line":"                    self._executor_state \u003c EXECUTOR_RUNNING):"},{"line_number":209,"context_line":"                # Executor is still initializing"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_7e2721ca","line":206,"in_reply_to":"5a2ca52d_9801f53a","updated":"2015-10-16 09:57:44.000000000","message":"ok let\u0027s fix the test please","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"0268bb6407d5e52d859c3430388838b3fe3ab5b2","unresolved":false,"context_lines":[{"line_number":203,"context_line":"        with self._state_cond:"},{"line_number":204,"context_line":"            # We must not call executor.stop() before executor.start() has"},{"line_number":205,"context_line":"            # completed. Wait for that here."},{"line_number":206,"context_line":"            executor \u003d self._executor"},{"line_number":207,"context_line":"            while (self._executor \u003d\u003d executor and"},{"line_number":208,"context_line":"                    self._executor_state \u003c EXECUTOR_RUNNING):"},{"line_number":209,"context_line":"                # Executor is still initializing"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_ceed82b8","line":206,"in_reply_to":"5a2ca52d_99bcbb07","updated":"2015-10-15 17:31:57.000000000","message":"Ya, I don\u0027t like this, I really want self._executor to never be set to none, recreated..., afaik this is trying to handle the case where stop has been called, but some other start has been called, and now the executor is different, to me this just shouldn\u0027t be done, don\u0027t reuse a server object like this (imho just make a new server object and start that instead...)\n\nHave a new state \u0027EXECUTOR_DEAD\u0027 and never let start() to move out of that one... and then we can cut a bunch of this code out imho...","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"78db45db01f1c19b0d85bddd4d650e63dc258f39","unresolved":false,"context_lines":[{"line_number":203,"context_line":"        with self._state_cond:"},{"line_number":204,"context_line":"            # We must not call executor.stop() before executor.start() has"},{"line_number":205,"context_line":"            # completed. Wait for that here."},{"line_number":206,"context_line":"            executor \u003d self._executor"},{"line_number":207,"context_line":"            while (self._executor \u003d\u003d executor and"},{"line_number":208,"context_line":"                    self._executor_state \u003c EXECUTOR_RUNNING):"},{"line_number":209,"context_line":"                # Executor is still initializing"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_9801f53a","line":206,"in_reply_to":"5a2ca52d_ceed82b8","updated":"2015-10-16 09:30:20.000000000","message":"This was my first pass implementation. State finished with EXECUTOR_WAITED. However, there\u0027s at least one test which calls start again, expecting it to restart. I\u0027ll try to find out which one so we can discuss whether or not we want this. I don\u0027t mind either way, and I agree that not wrapping would be cleaner.","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"bdc1c216a9aedd9e9bfb8b7417310a43160e29b2","unresolved":false,"context_lines":[{"line_number":207,"context_line":"            while (self._executor \u003d\u003d executor and"},{"line_number":208,"context_line":"                    self._executor_state \u003c EXECUTOR_RUNNING):"},{"line_number":209,"context_line":"                # Executor is still initializing"},{"line_number":210,"context_line":"                self._state_cond.wait()"},{"line_number":211,"context_line":""},{"line_number":212,"context_line":"            # We raced to call stop. The job has already been done, or is"},{"line_number":213,"context_line":"            # being handled by another thread."}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_19242b5e","line":210,"updated":"2015-10-15 16:10:50.000000000","message":"we hold the self._state_cond lock and we call wait() on it?","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"0268bb6407d5e52d859c3430388838b3fe3ab5b2","unresolved":false,"context_lines":[{"line_number":207,"context_line":"            while (self._executor \u003d\u003d executor and"},{"line_number":208,"context_line":"                    self._executor_state \u003c EXECUTOR_RUNNING):"},{"line_number":209,"context_line":"                # Executor is still initializing"},{"line_number":210,"context_line":"                self._state_cond.wait()"},{"line_number":211,"context_line":""},{"line_number":212,"context_line":"            # We raced to call stop. The job has already been done, or is"},{"line_number":213,"context_line":"            # being handled by another thread."}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_8e19cad9","line":210,"in_reply_to":"5a2ca52d_19242b5e","updated":"2015-10-15 17:31:57.000000000","message":"right, that releases the lock, puts the calling thread to sleep until some other thread notifies it to wake up.","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"ea59860695bb93e9ed778582e4da56a45fe9cd14","unresolved":false,"context_lines":[{"line_number":207,"context_line":"            while (self._executor \u003d\u003d executor and"},{"line_number":208,"context_line":"                    self._executor_state \u003c EXECUTOR_RUNNING):"},{"line_number":209,"context_line":"                # Executor is still initializing"},{"line_number":210,"context_line":"                self._state_cond.wait()"},{"line_number":211,"context_line":""},{"line_number":212,"context_line":"            # We raced to call stop. The job has already been done, or is"},{"line_number":213,"context_line":"            # being handled by another thread."}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_32447db5","line":210,"in_reply_to":"5a2ca52d_322f9d89","updated":"2015-10-16 10:12:09.000000000","message":"I thought you might say that :) When, though? Is there any code which actually expects to wait for a long period of time? For example, code which fires off a thread immediately after calling start() which calls wait() followed by some local cleanup (I don\u0027t know if this makes sense, btw), so might reasonably expect to wait for days/weeks?\n\nIf code calling this doesn\u0027t typically expect to wait long, 60 seconds seems like a reasonable amount of time to not trip up accidentally too often in production, whilst not being so long that somebody debugging will have killed it in frustration by then, and not get the benefit of it.","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"a92256fa406ad8abb3b7e543088a5c9906c0ab1d","unresolved":false,"context_lines":[{"line_number":207,"context_line":"            while (self._executor \u003d\u003d executor and"},{"line_number":208,"context_line":"                    self._executor_state \u003c EXECUTOR_RUNNING):"},{"line_number":209,"context_line":"                # Executor is still initializing"},{"line_number":210,"context_line":"                self._state_cond.wait()"},{"line_number":211,"context_line":""},{"line_number":212,"context_line":"            # We raced to call stop. The job has already been done, or is"},{"line_number":213,"context_line":"            # being handled by another thread."}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_802f2987","line":210,"in_reply_to":"5a2ca52d_32447db5","updated":"2015-10-16 15:18:22.000000000","message":"So how I\u0027ve handled it elsewhere is that stop and wait and similar always take a timeout, if that timeout is reached they return false (or raise an exception saying it timed out waiting); for example:\n\nhttps://github.com/openstack/taskflow/blob/master/taskflow/conductors/backends/impl_blocking.py#L228\n\nThen u can put an upper bound on this stop() and let the user decide how long they should wait for... (defaulting to infinite or none...)","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"ca1faf04a584c00f655144bc8cfdd574fa37b9df","unresolved":false,"context_lines":[{"line_number":207,"context_line":"            while (self._executor \u003d\u003d executor and"},{"line_number":208,"context_line":"                    self._executor_state \u003c EXECUTOR_RUNNING):"},{"line_number":209,"context_line":"                # Executor is still initializing"},{"line_number":210,"context_line":"                self._state_cond.wait()"},{"line_number":211,"context_line":""},{"line_number":212,"context_line":"            # We raced to call stop. The job has already been done, or is"},{"line_number":213,"context_line":"            # being handled by another thread."}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_322f9d89","line":210,"in_reply_to":"5a2ca52d_387f81ac","updated":"2015-10-16 09:57:44.000000000","message":"we should log stuff as we will get yelled at :)","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"ea66dfbb3df12a039c9492954f039b21a9ceef72","unresolved":false,"context_lines":[{"line_number":207,"context_line":"            while (self._executor \u003d\u003d executor and"},{"line_number":208,"context_line":"                    self._executor_state \u003c EXECUTOR_RUNNING):"},{"line_number":209,"context_line":"                # Executor is still initializing"},{"line_number":210,"context_line":"                self._state_cond.wait()"},{"line_number":211,"context_line":""},{"line_number":212,"context_line":"            # We raced to call stop. The job has already been done, or is"},{"line_number":213,"context_line":"            # being handled by another thread."}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_aefa8641","line":210,"in_reply_to":"5a2ca52d_8e19cad9","updated":"2015-10-15 17:32:56.000000000","message":"https://docs.python.org/2/library/threading.html#threading.Condition.wait","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"78db45db01f1c19b0d85bddd4d650e63dc258f39","unresolved":false,"context_lines":[{"line_number":207,"context_line":"            while (self._executor \u003d\u003d executor and"},{"line_number":208,"context_line":"                    self._executor_state \u003c EXECUTOR_RUNNING):"},{"line_number":209,"context_line":"                # Executor is still initializing"},{"line_number":210,"context_line":"                self._state_cond.wait()"},{"line_number":211,"context_line":""},{"line_number":212,"context_line":"            # We raced to call stop. The job has already been done, or is"},{"line_number":213,"context_line":"            # being handled by another thread."}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_387f81ac","line":210,"in_reply_to":"5a2ca52d_aefa8641","updated":"2015-10-16 09:30:20.000000000","message":"Right, holding the lock is a hard requirement for calling wait() (you get an exception if you\u0027re not holding it). It releases it before sleeping and retakes before waking in a race-free manner.\n\nHowever, how do we feel about indefinite waits in general? If the caller gets it wrong, they\u0027re just going to get a hang. Is there any heuristic we can apply so we could raise an exception after a timeout? If we got it right, it would be nicer to people debugging their code. If we got it wrong, though, we could bomb out just because we\u0027re busy. Perhaps a single log message at error level after 60 seconds, which includes a backtrace and current state, but then we continue waiting indefinitely? Would we ever expect a thread to wait longer than that in normal use?\n\nI will leave the waits indefinite with no logging for the moment.","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"0268bb6407d5e52d859c3430388838b3fe3ab5b2","unresolved":false,"context_lines":[{"line_number":211,"context_line":""},{"line_number":212,"context_line":"            # We raced to call stop. The job has already been done, or is"},{"line_number":213,"context_line":"            # being handled by another thread."},{"line_number":214,"context_line":"            if (self._executor !\u003d executor or"},{"line_number":215,"context_line":"                    self._executor_state \u003e EXECUTOR_RUNNING):"},{"line_number":216,"context_line":"                return"},{"line_number":217,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_6e622e40","line":214,"updated":"2015-10-15 17:31:57.000000000","message":"Ya, meh, added complexity that I understand why its here, but IMHO we just need to stop people from calling start, stop, start, stop and doing this \u0027weirdness\u0027 in the first place...","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"ca1faf04a584c00f655144bc8cfdd574fa37b9df","unresolved":false,"context_lines":[{"line_number":211,"context_line":""},{"line_number":212,"context_line":"            # We raced to call stop. The job has already been done, or is"},{"line_number":213,"context_line":"            # being handled by another thread."},{"line_number":214,"context_line":"            if (self._executor !\u003d executor or"},{"line_number":215,"context_line":"                    self._executor_state \u003e EXECUTOR_RUNNING):"},{"line_number":216,"context_line":"                return"},{"line_number":217,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_7240b534","line":214,"in_reply_to":"5a2ca52d_6e622e40","updated":"2015-10-16 09:57:44.000000000","message":"agreeed","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"bdc1c216a9aedd9e9bfb8b7417310a43160e29b2","unresolved":false,"context_lines":[{"line_number":221,"context_line":"        # Only 1 thread will get here with _executor_state \u003d\u003d EXECUTOR_STOPPING"},{"line_number":222,"context_line":"        # Explicitly drop the _state_cond lock before calling out"},{"line_number":223,"context_line":"        try:"},{"line_number":224,"context_line":"            self._executor.stop()"},{"line_number":225,"context_line":"        finally:"},{"line_number":226,"context_line":"            with self._state_cond:"},{"line_number":227,"context_line":"                self._executor_state \u003d EXECUTOR_STOPPED"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_d9d0f321","line":224,"updated":"2015-10-15 16:10:50.000000000","message":"should this be current_executor?","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"ca1faf04a584c00f655144bc8cfdd574fa37b9df","unresolved":false,"context_lines":[{"line_number":221,"context_line":"        # Only 1 thread will get here with _executor_state \u003d\u003d EXECUTOR_STOPPING"},{"line_number":222,"context_line":"        # Explicitly drop the _state_cond lock before calling out"},{"line_number":223,"context_line":"        try:"},{"line_number":224,"context_line":"            self._executor.stop()"},{"line_number":225,"context_line":"        finally:"},{"line_number":226,"context_line":"            with self._state_cond:"},{"line_number":227,"context_line":"                self._executor_state \u003d EXECUTOR_STOPPED"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_726755e9","line":224,"in_reply_to":"5a2ca52d_6e7b0ef6","updated":"2015-10-16 09:57:44.000000000","message":"whole heartedly agree","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"0268bb6407d5e52d859c3430388838b3fe3ab5b2","unresolved":false,"context_lines":[{"line_number":221,"context_line":"        # Only 1 thread will get here with _executor_state \u003d\u003d EXECUTOR_STOPPING"},{"line_number":222,"context_line":"        # Explicitly drop the _state_cond lock before calling out"},{"line_number":223,"context_line":"        try:"},{"line_number":224,"context_line":"            self._executor.stop()"},{"line_number":225,"context_line":"        finally:"},{"line_number":226,"context_line":"            with self._state_cond:"},{"line_number":227,"context_line":"                self._executor_state \u003d EXECUTOR_STOPPED"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_6e7b0ef6","line":224,"in_reply_to":"5a2ca52d_d9d0f321","updated":"2015-10-15 17:31:57.000000000","message":"Probably, see note about removing the ability to start more than once above, imho we should do that, it removes a bunch of this junk if we just recommend that as the usage pattern.","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"bdc1c216a9aedd9e9bfb8b7417310a43160e29b2","unresolved":false,"context_lines":[{"line_number":238,"context_line":"        server are released (like closing useless network connections)."},{"line_number":239,"context_line":"        \"\"\""},{"line_number":240,"context_line":"        with self._state_cond:"},{"line_number":241,"context_line":"            executor \u003d self._executor"},{"line_number":242,"context_line":"            # We can\u0027t wait until we\u0027re stopped"},{"line_number":243,"context_line":"            while (self._executor \u003d\u003d executor and"},{"line_number":244,"context_line":"                    self._executor_state \u003c EXECUTOR_STOPPED):"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_999d9b68","line":241,"updated":"2015-10-15 16:10:50.000000000","message":"can we call this current_executor? since we already have self.executor and self._executor","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"ca1faf04a584c00f655144bc8cfdd574fa37b9df","unresolved":false,"context_lines":[{"line_number":238,"context_line":"        server are released (like closing useless network connections)."},{"line_number":239,"context_line":"        \"\"\""},{"line_number":240,"context_line":"        with self._state_cond:"},{"line_number":241,"context_line":"            executor \u003d self._executor"},{"line_number":242,"context_line":"            # We can\u0027t wait until we\u0027re stopped"},{"line_number":243,"context_line":"            while (self._executor \u003d\u003d executor and"},{"line_number":244,"context_line":"                    self._executor_state \u003c EXECUTOR_STOPPED):"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_92a231fb","line":241,"in_reply_to":"5a2ca52d_999d9b68","updated":"2015-10-16 09:57:44.000000000","message":"please address this matthew","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"bdc1c216a9aedd9e9bfb8b7417310a43160e29b2","unresolved":false,"context_lines":[{"line_number":260,"context_line":"        # Only 1 thread will get here with _executor_state \u003d\u003d EXECUTOR_WAITING"},{"line_number":261,"context_line":"        # Explicitly drop the _state_cond lock before calling out"},{"line_number":262,"context_line":"        try:"},{"line_number":263,"context_line":"            self._executor.wait()"},{"line_number":264,"context_line":"        finally:"},{"line_number":265,"context_line":"            # Close listener connection after processing all messages"},{"line_number":266,"context_line":"            self._executor.listener.cleanup()"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_79967f40","line":263,"updated":"2015-10-15 16:10:50.000000000","message":"should this be current_executor?","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"0268bb6407d5e52d859c3430388838b3fe3ab5b2","unresolved":false,"context_lines":[{"line_number":260,"context_line":"        # Only 1 thread will get here with _executor_state \u003d\u003d EXECUTOR_WAITING"},{"line_number":261,"context_line":"        # Explicitly drop the _state_cond lock before calling out"},{"line_number":262,"context_line":"        try:"},{"line_number":263,"context_line":"            self._executor.wait()"},{"line_number":264,"context_line":"        finally:"},{"line_number":265,"context_line":"            # Close listener connection after processing all messages"},{"line_number":266,"context_line":"            self._executor.listener.cleanup()"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_ae4cc6c8","line":263,"in_reply_to":"5a2ca52d_79967f40","updated":"2015-10-15 17:31:57.000000000","message":"likely.","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"ca1faf04a584c00f655144bc8cfdd574fa37b9df","unresolved":false,"context_lines":[{"line_number":260,"context_line":"        # Only 1 thread will get here with _executor_state \u003d\u003d EXECUTOR_WAITING"},{"line_number":261,"context_line":"        # Explicitly drop the _state_cond lock before calling out"},{"line_number":262,"context_line":"        try:"},{"line_number":263,"context_line":"            self._executor.wait()"},{"line_number":264,"context_line":"        finally:"},{"line_number":265,"context_line":"            # Close listener connection after processing all messages"},{"line_number":266,"context_line":"            self._executor.listener.cleanup()"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a2ca52d_52b1f945","line":263,"in_reply_to":"5a2ca52d_ae4cc6c8","updated":"2015-10-16 09:57:44.000000000","message":"ditto","commit_id":"96c33711fd473d0f01dd858f828736ec28e1566a"},{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"a0b172fab6775b7d732452fef84862f670a318e3","unresolved":false,"context_lines":[{"line_number":168,"context_line":"        current thread."},{"line_number":169,"context_line":"        \"\"\""},{"line_number":170,"context_line":"        with self._state_cond:"},{"line_number":171,"context_line":"            if self._state \u003e EXECUTOR_UNINITIALIZED:"},{"line_number":172,"context_line":"                return"},{"line_number":173,"context_line":"            self._state \u003d EXECUTOR_INITIALIZING"},{"line_number":174,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3a29b11f_7a96944d","line":171,"updated":"2015-10-20 11:00:34.000000000","message":"My main concern about this state machine is that it changes the current behavior of the public API (even if the current one have some issues).\n\nFor example, start/stop/start/wait/stop is no more allowed (I know this case is full of race issue :p, once you call start again, you can\u0027t cleanup the listener of the first run).","commit_id":"91eaa4596eb2057c5192e978f73b55efe2deb411"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"e9ccca2e8b5fb7400e46e6d7cbcd1b318a758df3","unresolved":false,"context_lines":[{"line_number":168,"context_line":"        current thread."},{"line_number":169,"context_line":"        \"\"\""},{"line_number":170,"context_line":"        with self._state_cond:"},{"line_number":171,"context_line":"            if self._state \u003e EXECUTOR_UNINITIALIZED:"},{"line_number":172,"context_line":"                return"},{"line_number":173,"context_line":"            self._state \u003d EXECUTOR_INITIALIZING"},{"line_number":174,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3a29b11f_5cdf7fd5","line":171,"in_reply_to":"3a29b11f_7a96944d","updated":"2015-10-23 09:46:34.000000000","message":"Sorry, I totally missed these comments yesterday!\n\nYup, I had the same concern, and the new patch restores this. It also works quite differently.\n\nHowever, the new patch is also going to change the behaviour of the above:\n\nstart: executor.start()\nstop:  executor.stop()\nstart: DOES NOTHING (executor has already started)\nwait:  executor.wait()\nstop:  BLOCKS waiting for executor to start again, and you\u0027ll get a log message about it after 30 seconds with a stack trace if you\u0027ve got debug turned on.\n\nSo it\u0027ll be broken differently, but at least it will tell you about it :) I don\u0027t think there\u0027s a sane way to handle this, tbh.","commit_id":"91eaa4596eb2057c5192e978f73b55efe2deb411"},{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"a0b172fab6775b7d732452fef84862f670a318e3","unresolved":false,"context_lines":[{"line_number":201,"context_line":"        with self._state_cond:"},{"line_number":202,"context_line":"            # Wait until start() has been called"},{"line_number":203,"context_line":"            self._wait_cond(lambda: self._state \u003e\u003d EXECUTOR_RUNNING,"},{"line_number":204,"context_line":"                            _LW(\u0027Waiting in stop() for another thread to call \u0027"},{"line_number":205,"context_line":"                                \u0027start() for longer than %i seconds\u0027)"},{"line_number":206,"context_line":"                            % self.LOG_AFTER_WAIT_SECS)"},{"line_number":207,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3a29b11f_c45a7183","line":204,"updated":"2015-10-20 11:00:34.000000000","message":"Why do you want to warn if we call stop twice in separate threads ?","commit_id":"91eaa4596eb2057c5192e978f73b55efe2deb411"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"e9ccca2e8b5fb7400e46e6d7cbcd1b318a758df3","unresolved":false,"context_lines":[{"line_number":201,"context_line":"        with self._state_cond:"},{"line_number":202,"context_line":"            # Wait until start() has been called"},{"line_number":203,"context_line":"            self._wait_cond(lambda: self._state \u003e\u003d EXECUTOR_RUNNING,"},{"line_number":204,"context_line":"                            _LW(\u0027Waiting in stop() for another thread to call \u0027"},{"line_number":205,"context_line":"                                \u0027start() for longer than %i seconds\u0027)"},{"line_number":206,"context_line":"                            % self.LOG_AFTER_WAIT_SECS)"},{"line_number":207,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3a29b11f_bcb49381","line":204,"in_reply_to":"3a29b11f_c45a7183","updated":"2015-10-23 09:46:34.000000000","message":"These messages persist in the new version. They\u0027re actually only produced if we waited too long. You can call stop from as many threads as you like, and it will only actually be called once. You\u0027ll only get log messages if a thread waits for longer than 30 seconds.","commit_id":"91eaa4596eb2057c5192e978f73b55efe2deb411"},{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"a0b172fab6775b7d732452fef84862f670a318e3","unresolved":false,"context_lines":[{"line_number":235,"context_line":"        with self._state_cond:"},{"line_number":236,"context_line":"            # Wait until stop() has been called"},{"line_number":237,"context_line":"            self._wait_cond(lambda: self._state \u003e\u003d EXECUTOR_STOPPED,"},{"line_number":238,"context_line":"                            _LW(\u0027Waiting in wait() for another thread to call \u0027"},{"line_number":239,"context_line":"                                \u0027stop() for longer than %i seconds\u0027)"},{"line_number":240,"context_line":"                            % self.LOG_AFTER_WAIT_SECS)"},{"line_number":241,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3a29b11f_64f83d4a","line":238,"updated":"2015-10-20 11:00:34.000000000","message":"I would like to remove this warning to allow application to do\n\n  start()\n  wait()\n\nand another thread run stop() when the application have to stop.\n\n(I known it\u0027s not really related to your change, so you can ignore)","commit_id":"91eaa4596eb2057c5192e978f73b55efe2deb411"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"e9ccca2e8b5fb7400e46e6d7cbcd1b318a758df3","unresolved":false,"context_lines":[{"line_number":235,"context_line":"        with self._state_cond:"},{"line_number":236,"context_line":"            # Wait until stop() has been called"},{"line_number":237,"context_line":"            self._wait_cond(lambda: self._state \u003e\u003d EXECUTOR_STOPPED,"},{"line_number":238,"context_line":"                            _LW(\u0027Waiting in wait() for another thread to call \u0027"},{"line_number":239,"context_line":"                                \u0027stop() for longer than %i seconds\u0027)"},{"line_number":240,"context_line":"                            % self.LOG_AFTER_WAIT_SECS)"},{"line_number":241,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3a29b11f_1c1ee76f","line":238,"in_reply_to":"3a29b11f_64f83d4a","updated":"2015-10-23 09:46:34.000000000","message":"Ah...\n\nActually I think I could add that to the new version without too much difficulty. Will look while I\u0027m addressing other comments. The caller would do:\n\nstart()\nwait(log\u003dFalse)\n\nThe behaviour would be identical, except that no log would be emitted after 30 seconds.","commit_id":"91eaa4596eb2057c5192e978f73b55efe2deb411"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"b17f3736cc6ac32df6068a30fb060081e596917c","unresolved":false,"context_lines":[{"line_number":136,"context_line":""},{"line_number":137,"context_line":"        super(MessageHandlingServer, self).__init__()"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"    def _wait_cond(self, condition, warn_msg):"},{"line_number":140,"context_line":"        \u0027\u0027\u0027Wait with self._state_cond until condition() is true. Write a log"},{"line_number":141,"context_line":"        message if condition() has not become true within LOG_AFTER_WAIT_SECS."},{"line_number":142,"context_line":"        \u0027\u0027\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a29b11f_0c4dd93d","line":139,"updated":"2015-10-20 17:41:55.000000000","message":"Perhaps call this \u0027condition_func\u0027 or something, almost thought this was a threading.Condition but its not :-P","commit_id":"06d51d264a40435f23541f29b7bf6c11894b7830"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"d404e7b3c788e9122178629f8ea1c3154d4e9b39","unresolved":false,"context_lines":[{"line_number":125,"context_line":"        if self._state \u003d\u003d _TaskState.INIT:"},{"line_number":126,"context_line":"            self._state \u003d _TaskState.RUNNING"},{"line_number":127,"context_line":"            # Note that nothing waits on RUNNING, so no need to notify"},{"line_number":128,"context_line":"            self._cond.release()"},{"line_number":129,"context_line":"            try:"},{"line_number":130,"context_line":"                post_fn \u003d fn()"},{"line_number":131,"context_line":"            finally:"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a29b11f_704874cc","line":128,"updated":"2015-10-22 17:58:17.000000000","message":"Whats up with the release without an acquire?\n\nOh I think I see, `_ordered` touches this, woah, that\u0027s a little confusing, can that be done a little differently to make this more clear?","commit_id":"6ed57d787dd459235543ad60a8342d99af55a751"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"d404e7b3c788e9122178629f8ea1c3154d4e9b39","unresolved":false,"context_lines":[{"line_number":138,"context_line":"                try:"},{"line_number":139,"context_line":"                    post_fn()"},{"line_number":140,"context_line":"                finally:"},{"line_number":141,"context_line":"                    self._cond.acquire()"},{"line_number":142,"context_line":"        elif self._state \u003d\u003d _TaskState.RUNNING:"},{"line_number":143,"context_line":"            self._wait(lambda: self._state \u003d\u003d _TaskState.RUNNING,"},{"line_number":144,"context_line":"                       \u0027%s has been waiting on another thread to complete for \u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a29b11f_30151c9b","line":141,"updated":"2015-10-22 17:58:17.000000000","message":"Same question as above, acquire without release?","commit_id":"6ed57d787dd459235543ad60a8342d99af55a751"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"d404e7b3c788e9122178629f8ea1c3154d4e9b39","unresolved":false,"context_lines":[{"line_number":157,"context_line":"                  executing this method."},{"line_number":158,"context_line":"    \u0027\u0027\u0027"},{"line_number":159,"context_line":"    if after is not None:"},{"line_number":160,"context_line":"        after \u003d after.__name__"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"    def _ordered(fn):"},{"line_number":163,"context_line":"        state \u003d fn.__name__"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a29b11f_f081e4c3","line":160,"updated":"2015-10-22 17:58:17.000000000","message":"Use https://github.com/openstack/oslo.utils/blob/master/oslo_utils/reflection.py#L93 ?","commit_id":"6ed57d787dd459235543ad60a8342d99af55a751"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"d404e7b3c788e9122178629f8ea1c3154d4e9b39","unresolved":false,"context_lines":[{"line_number":166,"context_line":"            with self._state_cond:"},{"line_number":167,"context_line":"                # We store the states we started with in case the state wraps"},{"line_number":168,"context_line":"                # on us while we\u0027re sleeping."},{"line_number":169,"context_line":"                states \u003d self._states"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"                # Wait for the given preceding state to complete"},{"line_number":172,"context_line":"                if after is not None:"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a29b11f_f09d640a","line":169,"updated":"2015-10-22 17:58:17.000000000","message":"So this just store a reference to a dict I think, that dict can be altered, is that ok? The comment seems to state that it matters if the states is altered, which unless u do something like states \u003d dict(self._states) it still can be altered underneath u.","commit_id":"6ed57d787dd459235543ad60a8342d99af55a751"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"c6a69a3013a2d265fe20d6d955a5c0f2341e0f75","unresolved":false,"context_lines":[{"line_number":220,"context_line":"    def _ordered(fn):"},{"line_number":221,"context_line":"        # Set an attribute on the method so we can find it later"},{"line_number":222,"context_line":"        setattr(fn, \u0027_ordered\u0027, True)"},{"line_number":223,"context_line":"        state \u003d fn.__name__"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"        return _OrderedTaskRunner.decorate_ordered(fn, state, after)"},{"line_number":226,"context_line":"    return _ordered"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a29b11f_ae976b39","line":223,"updated":"2015-10-23 15:58:26.000000000","message":"Incidentally, reflection.get_callable_name returns different results for bound and unbound methods. fn is unbound here, but bound when calling init_task_states(). IIUC, __name__ works in both python 2.7 and python 3. It\u0027s also consistent for both bound and unbound methods.","commit_id":"d700c382791b6352bb80a0dc455589085881669f"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"6be846ad7a2e30823d0bf2b9166146638aa30e2b","unresolved":false,"context_lines":[{"line_number":220,"context_line":"    def _ordered(fn):"},{"line_number":221,"context_line":"        # Set an attribute on the method so we can find it later"},{"line_number":222,"context_line":"        setattr(fn, \u0027_ordered\u0027, True)"},{"line_number":223,"context_line":"        state \u003d fn.__name__"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"        return _OrderedTaskRunner.decorate_ordered(fn, state, after)"},{"line_number":226,"context_line":"    return _ordered"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a29b11f_c6c64449","line":223,"in_reply_to":"3a29b11f_ae976b39","updated":"2015-10-23 23:43:43.000000000","message":"Okie dokie. We should probably note that in reflection docstring, pretty sure thats cause py2.x is dumb, ha.","commit_id":"d700c382791b6352bb80a0dc455589085881669f"}]}