)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"c59d2ee639ab07e1affaa63262390f5e15ca6528","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"601e773c_35b2a867","updated":"2024-10-09 12:55:49.000000000","message":"otherwise lgtm","commit_id":"15f9333524b98969001dc3ec84e128936ba21690"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"0f757db3071333b2e3c1a5568913ea58433fdb3d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"1186b803_6cacb954","updated":"2024-10-09 18:38:42.000000000","message":"I\u0027ll be honest I mostly skimmed the actual driver implementation as I know this is largely and adaptation of existing code in nodepool. Instead I tried to review the new code whcih is primarily tests and fixtures.\n\nI did find one thing that may create race condition problems in the test case which I noted inline but otherwise this looks how I expected it to.","commit_id":"7c802b4e888d83c6e2ef5d185a907c9ee9c1663d"}],"tests/unit/test_openstack_driver.py":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"0f757db3071333b2e3c1a5568913ea58433fdb3d","unresolved":true,"context_lines":[{"line_number":91,"context_line":"        client \u003d LauncherClient(self.zk_client, None)"},{"line_number":92,"context_line":"        request \u003d client.getRequest(request.uuid)"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        self.assertEqual(request.state, model.NodesetRequest.State.FULFILLED)"},{"line_number":95,"context_line":"        self.assertEqual(len(request.nodes), 1)"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"        client.acceptNodeset(request, nodeset)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9b604f43_caf2dea8","line":94,"updated":"2024-10-09 18:38:42.000000000","message":"Do we need to wait for settled or similar here to ensure the nodeset request state has moved to fulfilled instead of requested? I suspect in the test framework this is very fast but it could still race depending on thread scheduling?","commit_id":"7c802b4e888d83c6e2ef5d185a907c9ee9c1663d"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"eaecfdc7e8a942dd3ca67b72fd4732103f7e9dbf","unresolved":false,"context_lines":[{"line_number":91,"context_line":"        client \u003d LauncherClient(self.zk_client, None)"},{"line_number":92,"context_line":"        request \u003d client.getRequest(request.uuid)"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        self.assertEqual(request.state, model.NodesetRequest.State.FULFILLED)"},{"line_number":95,"context_line":"        self.assertEqual(len(request.nodes), 1)"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"        client.acceptNodeset(request, nodeset)"}],"source_content_type":"text/x-python","patch_set":4,"id":"fad0db3c_c1fc6849","line":94,"in_reply_to":"9b604f43_caf2dea8","updated":"2024-10-09 18:45:50.000000000","message":"The requestNodes method actually does wait for the result event for the request, so we should be stable here.  waitUntilSettled would actually be a noop because of the way requestNodes uses the pipeline lock to get the result event.","commit_id":"7c802b4e888d83c6e2ef5d185a907c9ee9c1663d"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"0f757db3071333b2e3c1a5568913ea58433fdb3d","unresolved":true,"context_lines":[{"line_number":95,"context_line":"        self.assertEqual(len(request.nodes), 1)"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"        client.acceptNodeset(request, nodeset)"},{"line_number":98,"context_line":"        self.waitUntilSettled()"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"        with testtools.ExpectedException(NoNodeError):"},{"line_number":101,"context_line":"            # Request should be gone"}],"source_content_type":"text/x-python","patch_set":4,"id":"4b58dd62_9930249a","line":98,"updated":"2024-10-09 18:38:42.000000000","message":"See above. We wait at this point after accepting the nodeset on the client side. This makes the refresh below non racy I think.","commit_id":"7c802b4e888d83c6e2ef5d185a907c9ee9c1663d"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"eaecfdc7e8a942dd3ca67b72fd4732103f7e9dbf","unresolved":false,"context_lines":[{"line_number":95,"context_line":"        self.assertEqual(len(request.nodes), 1)"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"        client.acceptNodeset(request, nodeset)"},{"line_number":98,"context_line":"        self.waitUntilSettled()"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"        with testtools.ExpectedException(NoNodeError):"},{"line_number":101,"context_line":"            # Request should be gone"}],"source_content_type":"text/x-python","patch_set":4,"id":"7ab78421_4c273ac9","line":98,"in_reply_to":"4b58dd62_9930249a","updated":"2024-10-09 18:45:50.000000000","message":"TBH, I\u0027m not sure the wait is needed here; I think the action we\u0027re checking below is actually synchronous.  Doesn\u0027t hurt though.","commit_id":"7c802b4e888d83c6e2ef5d185a907c9ee9c1663d"}],"zuul/launcher/server.py":[{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"c59d2ee639ab07e1affaa63262390f5e15ca6528","unresolved":true,"context_lines":[{"line_number":487,"context_line":""},{"line_number":488,"context_line":"                log.debug(\"Checking node %s\", node)"},{"line_number":489,"context_line":"                node.create_state_machine.advance()"},{"line_number":490,"context_line":"                print(\u0027JEB\u0027, node.create_state_machine.state)"},{"line_number":491,"context_line":"                if not node.create_state_machine.complete:"},{"line_number":492,"context_line":"                    self.wake_event.set()"},{"line_number":493,"context_line":"                    return"}],"source_content_type":"text/x-python","patch_set":3,"id":"2b99b2dc_5f24b686","line":490,"updated":"2024-10-09 12:55:49.000000000","message":"Debug leftover","commit_id":"15f9333524b98969001dc3ec84e128936ba21690"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"0864025ab529eb27d5b0e78be3fdad939906030d","unresolved":false,"context_lines":[{"line_number":487,"context_line":""},{"line_number":488,"context_line":"                log.debug(\"Checking node %s\", node)"},{"line_number":489,"context_line":"                node.create_state_machine.advance()"},{"line_number":490,"context_line":"                print(\u0027JEB\u0027, node.create_state_machine.state)"},{"line_number":491,"context_line":"                if not node.create_state_machine.complete:"},{"line_number":492,"context_line":"                    self.wake_event.set()"},{"line_number":493,"context_line":"                    return"}],"source_content_type":"text/x-python","patch_set":3,"id":"2f96f24c_4fd33e70","line":490,"in_reply_to":"2b99b2dc_5f24b686","updated":"2024-10-09 15:37:45.000000000","message":"Done","commit_id":"15f9333524b98969001dc3ec84e128936ba21690"}]}
