)]}'
{"tests/unit/test_event_queues.py":[{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"5dfa846bcaf159f0b82da4024c1149100e796d6d","unresolved":false,"context_lines":[{"line_number":106,"context_line":"    pass"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"class DummyDriver(Driver, TriggerInterface):"},{"line_number":110,"context_line":"    name \u003d driver_name \u003d \"dummy\""},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    def getTrigger(self, connection, config\u003dNone):"}],"source_content_type":"text/x-python","patch_set":31,"id":"953eb4ac_0de972eb","line":109,"updated":"2021-03-15 18:33:14.000000000","message":"Below I\u0027m suggesting we use real management events because they are simple and low-cost, but here I think it makes sense to test with a dummy driver so we can avoid the overhead and distraction of actual driver behavior (that will be tested later anyway).","commit_id":"bd7ffc3671bec51ee1172a4d2817487db806c293"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"ce4cd7a70fee4e00c6bd86c2350394e93e9145cd","unresolved":false,"context_lines":[{"line_number":106,"context_line":"    pass"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"class DummyDriver(Driver, TriggerInterface):"},{"line_number":110,"context_line":"    name \u003d driver_name \u003d \"dummy\""},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    def getTrigger(self, connection, config\u003dNone):"}],"source_content_type":"text/x-python","patch_set":31,"id":"460f6207_c721750d","line":109,"in_reply_to":"953eb4ac_0de972eb","updated":"2021-03-16 06:25:13.000000000","message":"ack","commit_id":"bd7ffc3671bec51ee1172a4d2817487db806c293"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"5dfa846bcaf159f0b82da4024c1149100e796d6d","unresolved":false,"context_lines":[{"line_number":126,"context_line":"        self.driver \u003d DummyDriver()"},{"line_number":127,"context_line":"        self.connections.registerDriver(self.driver)"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    def test_global_trigger_events(self):"},{"line_number":130,"context_line":"        queue \u003d event_queues.GlobalTriggerEventQueue("},{"line_number":131,"context_line":"            self.zk_client, self.connections"},{"line_number":132,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":31,"id":"ffda610b_3ae95d1e","line":129,"updated":"2021-03-15 18:33:14.000000000","message":"We should have a comment about what the test is trying to ascertain.  I will try to add those; you can tell me if I\u0027m wrong.","commit_id":"bd7ffc3671bec51ee1172a4d2817487db806c293"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"ce4cd7a70fee4e00c6bd86c2350394e93e9145cd","unresolved":false,"context_lines":[{"line_number":126,"context_line":"        self.driver \u003d DummyDriver()"},{"line_number":127,"context_line":"        self.connections.registerDriver(self.driver)"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    def test_global_trigger_events(self):"},{"line_number":130,"context_line":"        queue \u003d event_queues.GlobalTriggerEventQueue("},{"line_number":131,"context_line":"            self.zk_client, self.connections"},{"line_number":132,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":31,"id":"fd27e2fd_b47245ee","line":129,"in_reply_to":"ffda610b_3ae95d1e","updated":"2021-03-16 06:25:13.000000000","message":"ok, thanks.","commit_id":"bd7ffc3671bec51ee1172a4d2817487db806c293"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"5dfa846bcaf159f0b82da4024c1149100e796d6d","unresolved":false,"context_lines":[{"line_number":187,"context_line":""},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"@patch.dict(event_queues.MANAGEMENT_EVENT_TYPE_MAP,"},{"line_number":190,"context_line":"            {\"DummyManagementEvent\": DummyManagementEvent})"},{"line_number":191,"context_line":"class TestManagementEventQueue(EventQueueBaseTestCase):"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    def test_management_events(self):"}],"source_content_type":"text/x-python","patch_set":31,"id":"dc7a05f3_1f975b97","line":190,"updated":"2021-03-15 18:33:14.000000000","message":"We can use real management events, which not only makes this simpler but gives more confidence in the actual code.  The use of a non-serializable \u0027config\u0027 value as discussed in a previous change would be a problem here, except that we can set that to None which is json serializable.","commit_id":"bd7ffc3671bec51ee1172a4d2817487db806c293"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"ce4cd7a70fee4e00c6bd86c2350394e93e9145cd","unresolved":false,"context_lines":[{"line_number":187,"context_line":""},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"@patch.dict(event_queues.MANAGEMENT_EVENT_TYPE_MAP,"},{"line_number":190,"context_line":"            {\"DummyManagementEvent\": DummyManagementEvent})"},{"line_number":191,"context_line":"class TestManagementEventQueue(EventQueueBaseTestCase):"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    def test_management_events(self):"}],"source_content_type":"text/x-python","patch_set":31,"id":"6f1ced9a_66fa27e2","line":190,"in_reply_to":"dc7a05f3_1f975b97","updated":"2021-03-16 06:25:13.000000000","message":"ack","commit_id":"bd7ffc3671bec51ee1172a4d2817487db806c293"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"5dfa846bcaf159f0b82da4024c1149100e796d6d","unresolved":false,"context_lines":[{"line_number":274,"context_line":"            pipeline_queue.put(event)"},{"line_number":275,"context_line":"            global_queue.ackWithoutResult(event)"},{"line_number":276,"context_line":""},{"line_number":277,"context_line":"        # Event was just forwarded and should not be acked"},{"line_number":278,"context_line":"        self.assertFalse(result_future.wait(0.1))"},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        self.assertEqual(len(global_queue), 0)"}],"source_content_type":"text/x-python","patch_set":31,"id":"2378e63a_12c4f27c","line":277,"updated":"2021-03-15 18:33:14.000000000","message":"IIUC, the global event has technically been \"acked\", since the event record has been deleted, but it\u0027s been replaced by an un-acked pipeline event, and the result hasn\u0027t been created; I think this comment is a little misleading.  But it sort of depends on when you consider a management event to be \"acked\".  I got pretty hung up on this; would you consider this accurate?\n\n  Event was just forwarded and since we expect a result, the future should not be completed yet.\n\nI put that in my revision.","commit_id":"bd7ffc3671bec51ee1172a4d2817487db806c293"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"ce4cd7a70fee4e00c6bd86c2350394e93e9145cd","unresolved":false,"context_lines":[{"line_number":274,"context_line":"            pipeline_queue.put(event)"},{"line_number":275,"context_line":"            global_queue.ackWithoutResult(event)"},{"line_number":276,"context_line":""},{"line_number":277,"context_line":"        # Event was just forwarded and should not be acked"},{"line_number":278,"context_line":"        self.assertFalse(result_future.wait(0.1))"},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        self.assertEqual(len(global_queue), 0)"}],"source_content_type":"text/x-python","patch_set":31,"id":"3e79f050_4e9ede6f","line":277,"in_reply_to":"2378e63a_12c4f27c","updated":"2021-03-16 06:25:13.000000000","message":"Yes, my comment is very misleading. This is about the result not the ack.","commit_id":"bd7ffc3671bec51ee1172a4d2817487db806c293"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"5dfa846bcaf159f0b82da4024c1149100e796d6d","unresolved":false,"context_lines":[{"line_number":293,"context_line":""},{"line_number":294,"context_line":""},{"line_number":295,"context_line":"class DummyResultEvent(model.ResultEvent, DummyEvent):"},{"line_number":296,"context_line":"    pass"},{"line_number":297,"context_line":""},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"@patch.dict(event_queues.RESULT_EVENT_TYPE_MAP,"}],"source_content_type":"text/x-python","patch_set":31,"id":"a05c9980_29940ff2","line":296,"updated":"2021-03-15 18:33:14.000000000","message":"I\u0027d like to use real result events if we can, but right now we don\u0027t inherit from the new event interface, so we would be getting ahead of ourselves.  I\u0027m adding a TODO for us to look at that later.","commit_id":"bd7ffc3671bec51ee1172a4d2817487db806c293"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"ce4cd7a70fee4e00c6bd86c2350394e93e9145cd","unresolved":false,"context_lines":[{"line_number":293,"context_line":""},{"line_number":294,"context_line":""},{"line_number":295,"context_line":"class DummyResultEvent(model.ResultEvent, DummyEvent):"},{"line_number":296,"context_line":"    pass"},{"line_number":297,"context_line":""},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"@patch.dict(event_queues.RESULT_EVENT_TYPE_MAP,"}],"source_content_type":"text/x-python","patch_set":31,"id":"07932676_631a3b4b","line":296,"in_reply_to":"a05c9980_29940ff2","updated":"2021-03-16 06:25:13.000000000","message":"ack","commit_id":"bd7ffc3671bec51ee1172a4d2817487db806c293"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"b9b30f40b49ef044ba97e1293b75df50b4f67877","unresolved":false,"context_lines":[{"line_number":297,"context_line":""},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"@patch.dict(event_queues.RESULT_EVENT_TYPE_MAP,"},{"line_number":300,"context_line":"            {\"DummyResultEvent\": DummyResultEvent})"},{"line_number":301,"context_line":"class TestResultEventQueue(EventQueueBaseTestCase):"},{"line_number":302,"context_line":""},{"line_number":303,"context_line":"    def test_pipeline_result_events(self):"}],"source_content_type":"text/x-python","patch_set":31,"id":"be2855dd_96d29c11","line":300,"updated":"2021-03-15 21:18:52.000000000","message":"I was able to remove this patch and the test still passed because it was treated as an invalid event, so I\u0027m going to add some more assertions to the test to make sure that we actually process the events we think we should.","commit_id":"bd7ffc3671bec51ee1172a4d2817487db806c293"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"ce4cd7a70fee4e00c6bd86c2350394e93e9145cd","unresolved":false,"context_lines":[{"line_number":297,"context_line":""},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"@patch.dict(event_queues.RESULT_EVENT_TYPE_MAP,"},{"line_number":300,"context_line":"            {\"DummyResultEvent\": DummyResultEvent})"},{"line_number":301,"context_line":"class TestResultEventQueue(EventQueueBaseTestCase):"},{"line_number":302,"context_line":""},{"line_number":303,"context_line":"    def test_pipeline_result_events(self):"}],"source_content_type":"text/x-python","patch_set":31,"id":"0f178f3d_a6d4f37b","line":300,"in_reply_to":"be2855dd_96d29c11","updated":"2021-03-16 06:25:13.000000000","message":"ok, thanks","commit_id":"bd7ffc3671bec51ee1172a4d2817487db806c293"}],"zuul/lib/collections.py":[{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"4fea9911ab7fd006f5cdc9ea1701a0b5d21d8c21","unresolved":false,"context_lines":[{"line_number":12,"context_line":"# License for the specific language governing permissions and limitations"},{"line_number":13,"context_line":"# under the License."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"from typing import Callable, DefaultDict, TypeVar"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"T \u003d TypeVar(\"T\")"}],"source_content_type":"text/x-python","patch_set":29,"id":"20434308_7537bd2b","line":15,"updated":"2021-03-12 23:01:18.000000000","message":"Maybe just use the one from collections instead of typing?","commit_id":"dda214469206294b5fae6c7d3ca9a35a3c274db6"}],"zuul/zk/event_queues.py":[{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"e3b2786ba6144618a819a20581b2cb934ff641d3","unresolved":false,"context_lines":[{"line_number":38,"context_line":"from zuul.zk import ZooKeeperClient, ZooKeeperBase"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"if TYPE_CHECKING:"},{"line_number":41,"context_line":"    from zuul.lib import connections as conn"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"RESULT_EVENT_TYPE_MAP \u003d {"},{"line_number":44,"context_line":"    \"BuildCompletedEvent\": model.BuildCompletedEvent,"}],"source_content_type":"text/x-python","patch_set":29,"id":"f16a4de7_fac545d2","line":41,"updated":"2021-03-15 17:50:53.000000000","message":"I think you may be right; this is proving to be impractical.  The class and method annotations tend to need too many type annotations in initializers.  And we also wanted to avoid recursive annotations, which is proving difficult.  Finally, I don\u0027t think anyone has proposed generics before, but I do think they go beyond the limited type hints that we envisioned.","commit_id":"dda214469206294b5fae6c7d3ca9a35a3c274db6"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"4fea9911ab7fd006f5cdc9ea1701a0b5d21d8c21","unresolved":false,"context_lines":[{"line_number":38,"context_line":"from zuul.zk import ZooKeeperClient, ZooKeeperBase"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"if TYPE_CHECKING:"},{"line_number":41,"context_line":"    from zuul.lib import connections as conn"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"RESULT_EVENT_TYPE_MAP \u003d {"},{"line_number":44,"context_line":"    \"BuildCompletedEvent\": model.BuildCompletedEvent,"}],"source_content_type":"text/x-python","patch_set":29,"id":"e6571d2e_c6359f69","line":41,"updated":"2021-03-12 23:01:18.000000000","message":"Let\u0027s prune the type checking from this file.","commit_id":"dda214469206294b5fae6c7d3ca9a35a3c274db6"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"a27e44b2b26a778f6a2d17841620dd2ab237bf68","unresolved":false,"context_lines":[{"line_number":38,"context_line":"from zuul.zk import ZooKeeperClient, ZooKeeperBase"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"if TYPE_CHECKING:"},{"line_number":41,"context_line":"    from zuul.lib import connections as conn"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"RESULT_EVENT_TYPE_MAP \u003d {"},{"line_number":44,"context_line":"    \"BuildCompletedEvent\": model.BuildCompletedEvent,"}],"source_content_type":"text/x-python","patch_set":29,"id":"ed6f37df_d789a4c4","line":41,"in_reply_to":"e6571d2e_c6359f69","updated":"2021-03-15 15:04:05.000000000","message":"I think it might make sense to take a stance and get rid of type annotations and type checking in Zuul completely. I have a different opinion on the usefulness of the annotations, but would prefer to have a clear NO instead of the current state where I don\u0027t know when it\u0027s ok to add type hints and when to leave them out.\n\nI tried to remove all type annotations from the method bodies and only kept the class + method level annotations, which I though was what we agreed on.","commit_id":"dda214469206294b5fae6c7d3ca9a35a3c274db6"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"582d4837866004e95e8fe0eda612d5baca4f1d3c","unresolved":false,"context_lines":[{"line_number":48,"context_line":"TENANT_ROOT \u003d \"/zuul/events/tenant\""},{"line_number":49,"context_line":"SCHEDULER_GLOBAL_ROOT \u003d \"/zuul/events/scheduler-global\""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"EventAckRef \u003d namedtuple(\"EventAckRef\", (\"path\", \"version\"))"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"UNKNOWN_ZVERSION \u003d -1"},{"line_number":54,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"9442ece0_94d6c277","line":51,"updated":"2021-03-12 23:13:08.000000000","message":"I\u0027m not really seeing how this is used yet.","commit_id":"0e637c56307da02a73b0bf2aa51ffc770a794de0"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"c7ebab3b958ed83885eeb45a2a83d26c720d3bd3","unresolved":false,"context_lines":[{"line_number":48,"context_line":"TENANT_ROOT \u003d \"/zuul/events/tenant\""},{"line_number":49,"context_line":"SCHEDULER_GLOBAL_ROOT \u003d \"/zuul/events/scheduler-global\""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"EventAckRef \u003d namedtuple(\"EventAckRef\", (\"path\", \"version\"))"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"UNKNOWN_ZVERSION \u003d -1"},{"line_number":54,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"b08bfc4b_eda33093","line":51,"updated":"2021-03-15 17:03:32.000000000","message":"Okay, so it\u0027s a reference to the event for use when acking, not a reference to an \"event ack\".  I was expecting it to be the latter, and could not find an ack ref attribute on the serialized version of the event in the queue.  That\u0027s because it\u0027s an attribute that\u0027s added after the fact to the de-serialized object.\n\nI\u0027ll add some documentation about this.","commit_id":"0e637c56307da02a73b0bf2aa51ffc770a794de0"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"9d7dd1214dd37d0a2361d1feeba67c053834b016","unresolved":false,"context_lines":[{"line_number":48,"context_line":"TENANT_ROOT \u003d \"/zuul/events/tenant\""},{"line_number":49,"context_line":"SCHEDULER_GLOBAL_ROOT \u003d \"/zuul/events/scheduler-global\""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"EventAckRef \u003d namedtuple(\"EventAckRef\", (\"path\", \"version\"))"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"UNKNOWN_ZVERSION \u003d -1"},{"line_number":54,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"d02d2370_48b72ee3","line":51,"in_reply_to":"9442ece0_94d6c277","updated":"2021-03-15 14:26:04.000000000","message":"This is the information we set on the event when it\u0027s consumed from the queue in order to find it later on when the event is acked (see the ack() method implementation).","commit_id":"0e637c56307da02a73b0bf2aa51ffc770a794de0"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"4f42b7f211e683219d0156710cc3a43b11c7a9fc","unresolved":false,"context_lines":[{"line_number":215,"context_line":"            with suppress(ValueError):"},{"line_number":216,"context_line":"                other_event \u003d event_list[event_list.index(event)]"},{"line_number":217,"context_line":"                if isinstance(other_event, model.TenantReconfigureEvent):"},{"line_number":218,"context_line":"                    other_event.merge(event)"},{"line_number":219,"context_line":"                    continue"},{"line_number":220,"context_line":"            event_list.append(event)"},{"line_number":221,"context_line":"        yield from event_list"}],"source_content_type":"text/x-python","patch_set":30,"id":"7ee214f0_c8a853e3","line":218,"updated":"2021-03-15 17:41:08.000000000","message":"Ah, good, you added it in a later patchset.  I\u0027m guessing the test helped catch that.","commit_id":"0e637c56307da02a73b0bf2aa51ffc770a794de0"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"c7ebab3b958ed83885eeb45a2a83d26c720d3bd3","unresolved":false,"context_lines":[{"line_number":215,"context_line":"            with suppress(ValueError):"},{"line_number":216,"context_line":"                other_event \u003d event_list[event_list.index(event)]"},{"line_number":217,"context_line":"                if isinstance(other_event, model.TenantReconfigureEvent):"},{"line_number":218,"context_line":"                    other_event.merge(event)"},{"line_number":219,"context_line":"                    continue"},{"line_number":220,"context_line":"            event_list.append(event)"},{"line_number":221,"context_line":"        yield from event_list"}],"source_content_type":"text/x-python","patch_set":30,"id":"ba5d2af0_4546ae2b","line":218,"updated":"2021-03-15 17:03:32.000000000","message":"The merge method doesn\u0027t appear to update merged_events; we should make sure to test this case.","commit_id":"0e637c56307da02a73b0bf2aa51ffc770a794de0"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"5d68ed010f106150f5c002741b0cc59b8c159d49","unresolved":false,"context_lines":[{"line_number":215,"context_line":"            with suppress(ValueError):"},{"line_number":216,"context_line":"                other_event \u003d event_list[event_list.index(event)]"},{"line_number":217,"context_line":"                if isinstance(other_event, model.TenantReconfigureEvent):"},{"line_number":218,"context_line":"                    other_event.merge(event)"},{"line_number":219,"context_line":"                    continue"},{"line_number":220,"context_line":"            event_list.append(event)"},{"line_number":221,"context_line":"        yield from event_list"}],"source_content_type":"text/x-python","patch_set":30,"id":"5f768cf4_5c2dac41","line":218,"in_reply_to":"7ee214f0_c8a853e3","updated":"2021-03-16 06:44:12.000000000","message":"It did ;)","commit_id":"0e637c56307da02a73b0bf2aa51ffc770a794de0"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"582d4837866004e95e8fe0eda612d5baca4f1d3c","unresolved":false,"context_lines":[{"line_number":257,"context_line":""},{"line_number":258,"context_line":"    @classmethod"},{"line_number":259,"context_line":"    def _createRegistry(cls, client, tenant_name):"},{"line_number":260,"context_line":"        return DefaultKeyDict(lambda p: cls(client, tenant_name, p))"},{"line_number":261,"context_line":""},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"class GlobalManagementEventQueue(ManagementEventQueue):"}],"source_content_type":"text/x-python","patch_set":30,"id":"64056be9_f8ff51a3","line":260,"updated":"2021-03-12 23:13:08.000000000","message":"I don\u0027t know what the registry is.","commit_id":"0e637c56307da02a73b0bf2aa51ffc770a794de0"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"9d7dd1214dd37d0a2361d1feeba67c053834b016","unresolved":false,"context_lines":[{"line_number":257,"context_line":""},{"line_number":258,"context_line":"    @classmethod"},{"line_number":259,"context_line":"    def _createRegistry(cls, client, tenant_name):"},{"line_number":260,"context_line":"        return DefaultKeyDict(lambda p: cls(client, tenant_name, p))"},{"line_number":261,"context_line":""},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"class GlobalManagementEventQueue(ManagementEventQueue):"}],"source_content_type":"text/x-python","patch_set":30,"id":"6fa08d15_f0bb70fa","line":260,"in_reply_to":"64056be9_f8ff51a3","updated":"2021-03-15 14:26:04.000000000","message":"Yes, I think this makes only sense when looking at how the event queues are used in the scheduler. Basically the registry allows accessing a tenants pipeline event queue as follows:\n\n    queue_registry \u003d PipelineManagementEventQueue.createRegistry(self.zk_client)\n    assert isinstance(\n        queue_registry[\"tenant_name\"][\"pipeline_name\"],\n        PipelineManagementEventQueue\n    )","commit_id":"0e637c56307da02a73b0bf2aa51ffc770a794de0"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"e958244bb89b5cfed4053db0e60a8db812de0260","unresolved":true,"context_lines":[{"line_number":103,"context_line":"            makepath\u003dTrue,"},{"line_number":104,"context_line":"        )"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"    def _iterEvents(self):"},{"line_number":107,"context_line":"        try:"},{"line_number":108,"context_line":"            events \u003d self.kazoo_client.get_children(self.event_root)"},{"line_number":109,"context_line":"        except NoNodeError:"}],"source_content_type":"text/x-python","patch_set":31,"id":"3d76ef88_77330343","line":106,"updated":"2021-03-15 14:50:27.000000000","message":"I\u0027m honestly confused when to use camelCase vs snake_case in Zuul. I was under the impression that it\u0027s mostly camelCase, but snake_case for private methods. I tried to follow that style, (I might have used snake_case out of habit sometimes). From your update it now seems it should be camelCase for everything (except properties?!).","commit_id":"bd7ffc3671bec51ee1172a4d2817487db806c293"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"778ba338a32a84d9918deadb620e18a352996733","unresolved":false,"context_lines":[{"line_number":103,"context_line":"            makepath\u003dTrue,"},{"line_number":104,"context_line":"        )"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"    def _iterEvents(self):"},{"line_number":107,"context_line":"        try:"},{"line_number":108,"context_line":"            events \u003d self.kazoo_client.get_children(self.event_root)"},{"line_number":109,"context_line":"        except NoNodeError:"}],"source_content_type":"text/x-python","patch_set":31,"id":"a24d10a1_0a7ab657","line":106,"updated":"2021-03-15 17:13:58.000000000","message":"Yeah, it\u0027s not perfectly consistent, sorry.  :(  It\u0027s supposed to be camelCase for everything.  Sometimes things slip through and it\u0027s admittedly less important for internal methods that are only used in the same file.  I think it was the public \"create_registry\" method that made me think the rework was worth doing.\n\nAnd, yes, this essentially follows the old Java convention: CapitalClasses.camelCaseMethods() and .snake_case_attributes.  That was fairly common in python 1.5.2 days and not unheard of in the early 2.x versions.  People don\u0027t acknowledge that now though.  Anyway, if we started from scratch or decided to do a major reformatting, I\u0027d go with the current style.  But in the mean time, consistency, or at least as close as we can get, is worth it.","commit_id":"bd7ffc3671bec51ee1172a4d2817487db806c293"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"ce4cd7a70fee4e00c6bd86c2350394e93e9145cd","unresolved":false,"context_lines":[{"line_number":103,"context_line":"            makepath\u003dTrue,"},{"line_number":104,"context_line":"        )"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"    def _iterEvents(self):"},{"line_number":107,"context_line":"        try:"},{"line_number":108,"context_line":"            events \u003d self.kazoo_client.get_children(self.event_root)"},{"line_number":109,"context_line":"        except NoNodeError:"}],"source_content_type":"text/x-python","patch_set":31,"id":"765b69cd_58f79f72","line":106,"in_reply_to":"a24d10a1_0a7ab657","updated":"2021-03-16 06:25:13.000000000","message":"Ok, thanks for clarifying.","commit_id":"bd7ffc3671bec51ee1172a4d2817487db806c293"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"b9b30f40b49ef044ba97e1293b75df50b4f67877","unresolved":false,"context_lines":[{"line_number":222,"context_line":""},{"line_number":223,"context_line":"    def ack(self, event):"},{"line_number":224,"context_line":"        super().ack(event)"},{"line_number":225,"context_line":"        self._reportResult(event)"},{"line_number":226,"context_line":"        if isinstance(event, model.TenantReconfigureEvent):"},{"line_number":227,"context_line":"            for merged_event in event.merged_events:"},{"line_number":228,"context_line":"                super().ack(merged_event)"}],"source_content_type":"text/x-python","patch_set":31,"id":"f250dbe5_e39aa8c5","line":225,"updated":"2021-03-15 21:18:52.000000000","message":"This deletes the original event before reporting the result, so if the scheduler crashes between these two lines, the caller will wait forever.  I think it may be safer to reverse the order.  I\u0027m going to go ahead and do that in my revision, but this is pretty delicate, so I want to make sure you agree.  I\u0027ll leave a note about the importance of it.","commit_id":"bd7ffc3671bec51ee1172a4d2817487db806c293"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"ce4cd7a70fee4e00c6bd86c2350394e93e9145cd","unresolved":false,"context_lines":[{"line_number":222,"context_line":""},{"line_number":223,"context_line":"    def ack(self, event):"},{"line_number":224,"context_line":"        super().ack(event)"},{"line_number":225,"context_line":"        self._reportResult(event)"},{"line_number":226,"context_line":"        if isinstance(event, model.TenantReconfigureEvent):"},{"line_number":227,"context_line":"            for merged_event in event.merged_events:"},{"line_number":228,"context_line":"                super().ack(merged_event)"}],"source_content_type":"text/x-python","patch_set":31,"id":"8546e5c8_b5b2985e","line":225,"in_reply_to":"f250dbe5_e39aa8c5","updated":"2021-03-16 06:25:13.000000000","message":"Yes, good catch. I think it\u0027s better for the result to be set twice in the case the event processing is interrupted than to let the caller run into a timeout (or worse hang forever).","commit_id":"bd7ffc3671bec51ee1172a4d2817487db806c293"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"20bb09d67844b9da2b135e794a5bf441c06d0208","unresolved":false,"context_lines":[{"line_number":239,"context_line":"            json.dumps(result_data).encode(\"utf-8\"),"},{"line_number":240,"context_line":"            ephemeral\u003dTrue,"},{"line_number":241,"context_line":"            makepath\u003dTrue,"},{"line_number":242,"context_line":"        )"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":""},{"line_number":245,"context_line":"class PipelineManagementEventQueue(ManagementEventQueue):"}],"source_content_type":"text/x-python","patch_set":31,"id":"483e3b6d_d5029e5c","line":242,"updated":"2021-03-17 00:06:53.000000000","message":"I like that suggestion; we may have other things that need garbage collecting.  I don\u0027t think we would need to run this very often (maybe every hour?) and it could delete things that are, say, \u003e24 old.","commit_id":"bd7ffc3671bec51ee1172a4d2817487db806c293"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"b9b30f40b49ef044ba97e1293b75df50b4f67877","unresolved":false,"context_lines":[{"line_number":239,"context_line":"            json.dumps(result_data).encode(\"utf-8\"),"},{"line_number":240,"context_line":"            ephemeral\u003dTrue,"},{"line_number":241,"context_line":"            makepath\u003dTrue,"},{"line_number":242,"context_line":"        )"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":""},{"line_number":245,"context_line":"class PipelineManagementEventQueue(ManagementEventQueue):"}],"source_content_type":"text/x-python","patch_set":31,"id":"79321640_c14983e0","line":242,"updated":"2021-03-15 21:18:52.000000000","message":"Why is the result created as an ephemeral node?  If the scheduler crashes between reporting the result and the client reading it, then the client won\u0027t see the result.\n\nIdeally we want the client to delete the event after deleting it in order to avoid having these stick around for the lifetime of the scheduler anyway.  But we can\u0027t rely on the client doing that, because someone may have hit CTRL-C.  So I think we should do the following:\n\n1) Don\u0027t create the nodes as ephemeral.\n2) Have the client delete the node (this is already the case).\n3) Periodically look for leaked result nodes and delete them.\n\nHere\u0027s an alternative (but I don\u0027t think we should do this):\n\nCurrently the client ignores the data in the result callback in favor of simply performing a ZK get on the znode.  If we used the data from the callback instead of re-reading it, we would not be subject to the race with the server crashing before the client reads the data.  That might be worth doing just to save a ZK read call, however, it still doesn\u0027t solve the issue of clients not cleaning up results and them sticking around until the server exits.  For that reason alone, I think we should drop the ephemeral flag and add a cleanup process.","commit_id":"bd7ffc3671bec51ee1172a4d2817487db806c293"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"ce4cd7a70fee4e00c6bd86c2350394e93e9145cd","unresolved":false,"context_lines":[{"line_number":239,"context_line":"            json.dumps(result_data).encode(\"utf-8\"),"},{"line_number":240,"context_line":"            ephemeral\u003dTrue,"},{"line_number":241,"context_line":"            makepath\u003dTrue,"},{"line_number":242,"context_line":"        )"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":""},{"line_number":245,"context_line":"class PipelineManagementEventQueue(ManagementEventQueue):"}],"source_content_type":"text/x-python","patch_set":31,"id":"05796cbc_06d580f2","line":242,"in_reply_to":"79321640_c14983e0","updated":"2021-03-16 06:25:13.000000000","message":"Should this cleanup procedure be something specific to the event queues or would some kind of scheduler global cleanup hook be better? Something like:\n\n    scheduler.register_cleanup_hook(mgmt_queue.cleanup)","commit_id":"bd7ffc3671bec51ee1172a4d2817487db806c293"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"b9b30f40b49ef044ba97e1293b75df50b4f67877","unresolved":false,"context_lines":[{"line_number":253,"context_line":""},{"line_number":254,"context_line":"    @classmethod"},{"line_number":255,"context_line":"    def createRegistry(cls, client):"},{"line_number":256,"context_line":"        return DefaultKeyDict(lambda t: cls._createRegistry(client, t))"},{"line_number":257,"context_line":""},{"line_number":258,"context_line":"    @classmethod"},{"line_number":259,"context_line":"    def _createRegistry(cls, client, tenant_name):"}],"source_content_type":"text/x-python","patch_set":31,"id":"660d54b8_47002594","line":256,"updated":"2021-03-15 21:18:52.000000000","message":"Based on your response to the earlier patchset and the code in the tests, I think I understand what this is now.  But if I\u0027m following it correctly, couldn\u0027t we use a dictionary keyed by tuple instead?","commit_id":"bd7ffc3671bec51ee1172a4d2817487db806c293"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"20bb09d67844b9da2b135e794a5bf441c06d0208","unresolved":false,"context_lines":[{"line_number":253,"context_line":""},{"line_number":254,"context_line":"    @classmethod"},{"line_number":255,"context_line":"    def createRegistry(cls, client):"},{"line_number":256,"context_line":"        return DefaultKeyDict(lambda t: cls._createRegistry(client, t))"},{"line_number":257,"context_line":""},{"line_number":258,"context_line":"    @classmethod"},{"line_number":259,"context_line":"    def _createRegistry(cls, client, tenant_name):"}],"source_content_type":"text/x-python","patch_set":31,"id":"d08091f8_719ce05d","line":256,"updated":"2021-03-17 00:06:53.000000000","message":"This is fine.  We need the defaultkeydict in order to pass in the reference to the client, so we may as well stick with this.  If we didn\u0027t need that, I would be in favor of simplifying it with a plain dict indexed by tuple.","commit_id":"bd7ffc3671bec51ee1172a4d2817487db806c293"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"ce4cd7a70fee4e00c6bd86c2350394e93e9145cd","unresolved":false,"context_lines":[{"line_number":253,"context_line":""},{"line_number":254,"context_line":"    @classmethod"},{"line_number":255,"context_line":"    def createRegistry(cls, client):"},{"line_number":256,"context_line":"        return DefaultKeyDict(lambda t: cls._createRegistry(client, t))"},{"line_number":257,"context_line":""},{"line_number":258,"context_line":"    @classmethod"},{"line_number":259,"context_line":"    def _createRegistry(cls, client, tenant_name):"}],"source_content_type":"text/x-python","patch_set":31,"id":"2a5f2803_b683482c","line":256,"in_reply_to":"660d54b8_47002594","updated":"2021-03-16 06:25:13.000000000","message":"Yes, we could. The API would look something like this:\n\n    event_queue[tenant.name, pipeline.name]\n\nPersonally I like the nested dict syntax better (tuples as keys looks a little unfamiliar to me), but it\u0027s an easy change in case you\u0027d prefer that.","commit_id":"bd7ffc3671bec51ee1172a4d2817487db806c293"},{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"fe7089dbe4dce5ec580159bd9b173ff1fe700375","unresolved":true,"context_lines":[{"line_number":207,"context_line":"                # TODO: raise some kind of ManagementEventException here"},{"line_number":208,"context_line":"                raise RuntimeError(tb)"},{"line_number":209,"context_line":"        finally:"},{"line_number":210,"context_line":"            with suppress(NoNodeError):"},{"line_number":211,"context_line":"                self.kazoo_client.delete(self._result_path)"},{"line_number":212,"context_line":"        return True"},{"line_number":213,"context_line":""}],"source_content_type":"text/x-python","patch_set":35,"id":"772a0975_a2e04423","line":210,"updated":"2021-03-19 19:52:56.000000000","message":"I like this method of suppressing a distinct exception :)","commit_id":"b23b9d7844fb00a3a731cfb192ce1a65c96d3a3f"}]}