)]}'
{"tests/base.py":[{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"420c0f6f1e6a4e88b1ff206fb2aa4ffbf02cb7b2","unresolved":false,"context_lines":[{"line_number":2167,"context_line":"        if not commit:  # merge conflict"},{"line_number":2168,"context_line":"            self.recordResult(\u0027MERGER_FAILURE\u0027)"},{"line_number":2169,"context_line":""},{"line_number":2170,"context_line":"        while self.executor_server.hold_jobs_in_start:"},{"line_number":2171,"context_line":"            time.sleep(1)"},{"line_number":2172,"context_line":""},{"line_number":2173,"context_line":"        return commit"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_79c0a47d","line":2170,"updated":"2019-10-21 16:34:24.000000000","message":"But if something goes wrong and the test aborts before that happens, this could get stuck here forever.  hold_jobs_in_build uses a Condition which is set in teardown to make sure it gets cleaned up.  We could do the same here, but this isn\u0027t going to be used that much so it\u0027s probably okay to use iterate_timeout.  You can set a long timeout.","commit_id":"8df57d47d239b4828dc59cc16d67f4c0a9f158c2"},{"author":{"_account_id":9311,"name":"Tristan Cacqueray","email":"tdecacqu@redhat.com","username":"tristanC"},"change_message_id":"7e60bb840fd8154a7aa3cf519dbae92d953abc46","unresolved":false,"context_lines":[{"line_number":2167,"context_line":"        if not commit:  # merge conflict"},{"line_number":2168,"context_line":"            self.recordResult(\u0027MERGER_FAILURE\u0027)"},{"line_number":2169,"context_line":""},{"line_number":2170,"context_line":"        while self.executor_server.hold_jobs_in_start:"},{"line_number":2171,"context_line":"            time.sleep(1)"},{"line_number":2172,"context_line":""},{"line_number":2173,"context_line":"        return commit"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_6d162958","line":2170,"updated":"2019-09-25 14:03:00.000000000","message":"why not using the iterate_timeout procedure?","commit_id":"8df57d47d239b4828dc59cc16d67f4c0a9f158c2"},{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"5bd43ce422fa66d35074d7f780ef390e91fad8c9","unresolved":false,"context_lines":[{"line_number":2167,"context_line":"        if not commit:  # merge conflict"},{"line_number":2168,"context_line":"            self.recordResult(\u0027MERGER_FAILURE\u0027)"},{"line_number":2169,"context_line":""},{"line_number":2170,"context_line":"        while self.executor_server.hold_jobs_in_start:"},{"line_number":2171,"context_line":"            time.sleep(1)"},{"line_number":2172,"context_line":""},{"line_number":2173,"context_line":"        return commit"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_f595fb87","line":2170,"in_reply_to":"3fa7e38b_6d162958","updated":"2019-09-25 15:13:03.000000000","message":"Because we don\u0027t need a timeout here. The test case explicitly releases this loop by resetting hold_jobs_in_start to False.","commit_id":"8df57d47d239b4828dc59cc16d67f4c0a9f158c2"}],"zuul/executor/client.py":[{"author":{"_account_id":9311,"name":"Tristan Cacqueray","email":"tdecacqu@redhat.com","username":"tristanC"},"change_message_id":"7e60bb840fd8154a7aa3cf519dbae92d953abc46","unresolved":false,"context_lines":[{"line_number":383,"context_line":"        except AttributeError:"},{"line_number":384,"context_line":"            log.debug(\"Build has no associated gearman job\")"},{"line_number":385,"context_line":"            return False"},{"line_number":386,"context_line":""},{"line_number":387,"context_line":"        if build.__gearman_worker is not None:"},{"line_number":388,"context_line":"            log.debug(\"Build has already started\")"},{"line_number":389,"context_line":"            self.cancelRunningBuild(build)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_9f19487b","line":386,"updated":"2019-09-25 14:03:00.000000000","message":"Perhaps create a build_worker variable similarly to the job one above so that the rest of the code doesn\u0027t reference a __ private attribute?","commit_id":"8df57d47d239b4828dc59cc16d67f4c0a9f158c2"},{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"5bd43ce422fa66d35074d7f780ef390e91fad8c9","unresolved":false,"context_lines":[{"line_number":383,"context_line":"        except AttributeError:"},{"line_number":384,"context_line":"            log.debug(\"Build has no associated gearman job\")"},{"line_number":385,"context_line":"            return False"},{"line_number":386,"context_line":""},{"line_number":387,"context_line":"        if build.__gearman_worker is not None:"},{"line_number":388,"context_line":"            log.debug(\"Build has already started\")"},{"line_number":389,"context_line":"            self.cancelRunningBuild(build)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_f53e5bb6","line":386,"in_reply_to":"3fa7e38b_9f19487b","updated":"2019-09-25 15:13:03.000000000","message":"The exact same variable is also used already by cancelRunningBuild and in some other places which I didn\u0027t want to refactor.","commit_id":"8df57d47d239b4828dc59cc16d67f4c0a9f158c2"},{"author":{"_account_id":9311,"name":"Tristan Cacqueray","email":"tdecacqu@redhat.com","username":"tristanC"},"change_message_id":"18525dabf09ed38e9d728f01ea6e622ebc726bfa","unresolved":false,"context_lines":[{"line_number":481,"context_line":"            build.url \u003d data.get(\u0027url\u0027, build.url)"},{"line_number":482,"context_line":"            # Update information about worker"},{"line_number":483,"context_line":"            build.worker.updateFromData(data)"},{"line_number":484,"context_line":"            build.__gearman_worker \u003d build.worker.name"},{"line_number":485,"context_line":""},{"line_number":486,"context_line":"            if \u0027paused\u0027 in data and build.paused !\u003d data[\u0027paused\u0027]:"},{"line_number":487,"context_line":"                build.paused \u003d data[\u0027paused\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_f5865bb9","line":484,"updated":"2019-09-25 15:38:37.000000000","message":"iirc this is used to indicate the build has started and thus can be aborted. Why not naming this attribute started then? Or add a started property to the build object that check for the __gearman_worker","commit_id":"8df57d47d239b4828dc59cc16d67f4c0a9f158c2"},{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"ab6c1e885fc468182eca284181860427b970f94d","unresolved":false,"context_lines":[{"line_number":481,"context_line":"            build.url \u003d data.get(\u0027url\u0027, build.url)"},{"line_number":482,"context_line":"            # Update information about worker"},{"line_number":483,"context_line":"            build.worker.updateFromData(data)"},{"line_number":484,"context_line":"            build.__gearman_worker \u003d build.worker.name"},{"line_number":485,"context_line":""},{"line_number":486,"context_line":"            if \u0027paused\u0027 in data and build.paused !\u003d data[\u0027paused\u0027]:"},{"line_number":487,"context_line":"                build.paused \u003d data[\u0027paused\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_008de5d1","line":484,"in_reply_to":"3fa7e38b_f5865bb9","updated":"2019-09-26 12:20:47.000000000","message":"This attribute must be gearman_worker as this is not only used to determine if the build has started but also to which executor stop jobs need to be sent. So gearman_worker really is correct. What we could do however is to remove this variable completely and use build.worker.name directly which contains the same information anyway. (but maybe in a different change as this is an unrelated refactoring?)","commit_id":"8df57d47d239b4828dc59cc16d67f4c0a9f158c2"}],"zuul/executor/server.py":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"9bcdafef2f940930e3f7bfe195d14d7002195626","unresolved":false,"context_lines":[{"line_number":1017,"context_line":"            self._send_aborted()"},{"line_number":1018,"context_line":"            return"},{"line_number":1019,"context_line":""},{"line_number":1020,"context_line":"        data \u003d {}"},{"line_number":1021,"context_line":"        if self.executor_server.log_streaming_port !\u003d DEFAULT_FINGER_PORT:"},{"line_number":1022,"context_line":"            data[\u0027url\u0027] \u003d \"finger://{hostname}:{port}/{uuid}\".format("},{"line_number":1023,"context_line":"                hostname\u003dself.executor_server.hostname,"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_add5ace7","line":1020,"updated":"2019-11-25 23:56:51.000000000","message":"I think this may introduce a race. getJobData() always looks at the last entry of the gearman Job\u0027s data list. The gear lib simply appends each WORK_DATA packet that it receives to this list.\n\nWhat this means is if the executor client receives the previous WORK_DATA with worker_hostname in it then receives this WORK_DATA with url in it before processing the first WORK_DATA response the client will only see the url work data and not see the worker_hostname data. It will then fail in Worker.updateFromData().\n\nTo address this I think we can simply continue to provide the old data here as well then the most recent WORK_DATA entry will include as much info as possible.","commit_id":"0b6850c9488ca5a19f1b41158669588261822069"},{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"bbfb0ff70572c776cc4072244f5cb9d734bfef7b","unresolved":false,"context_lines":[{"line_number":1017,"context_line":"            self._send_aborted()"},{"line_number":1018,"context_line":"            return"},{"line_number":1019,"context_line":""},{"line_number":1020,"context_line":"        data \u003d {}"},{"line_number":1021,"context_line":"        if self.executor_server.log_streaming_port !\u003d DEFAULT_FINGER_PORT:"},{"line_number":1022,"context_line":"            data[\u0027url\u0027] \u003d \"finger://{hostname}:{port}/{uuid}\".format("},{"line_number":1023,"context_line":"                hostname\u003dself.executor_server.hostname,"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_47fada76","line":1020,"in_reply_to":"3fa7e38b_add5ace7","updated":"2019-12-13 11:53:09.000000000","message":"Done","commit_id":"0b6850c9488ca5a19f1b41158669588261822069"}]}
