)]}'
{"zuul/executor/client.py":[{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"9aef41d01e0536571a7919d8289fa731f857dfc9","unresolved":true,"context_lines":[{"line_number":113,"context_line":"        executor_zone \u003d None"},{"line_number":114,"context_line":"        if len(nodeset.nodes):"},{"line_number":115,"context_line":"            node \u003d nodeset.getNodes()[0]"},{"line_number":116,"context_line":"            if hasattr(node, \u0027attributes\u0027) and node.attributes:"},{"line_number":117,"context_line":"                executor_zone \u003d node.attributes.get(\u0027executor-zone\u0027)"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"        zone_known \u003d False"}],"source_content_type":"text/x-python","patch_set":5,"id":"346a043a_7a6b3193","line":116,"updated":"2021-09-01 13:40:43.000000000","message":"Just wondering what this optional \"attributes\" attr of a node is? Something added by nodepool?  If yes, can we maybe initialize that in our Node class to an empty dict?","commit_id":"39fb64a6d856af3e741c99a8b13eb712df627626"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"86c374c4cce8553b9f5df99051fe5becd7a3aa0a","unresolved":false,"context_lines":[{"line_number":113,"context_line":"        executor_zone \u003d None"},{"line_number":114,"context_line":"        if len(nodeset.nodes):"},{"line_number":115,"context_line":"            node \u003d nodeset.getNodes()[0]"},{"line_number":116,"context_line":"            if hasattr(node, \u0027attributes\u0027) and node.attributes:"},{"line_number":117,"context_line":"                executor_zone \u003d node.attributes.get(\u0027executor-zone\u0027)"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"        zone_known \u003d False"}],"source_content_type":"text/x-python","patch_set":5,"id":"5500a96a_a43a18ef","line":116,"updated":"2021-09-02 02:28:01.000000000","message":"Yep.  I\u0027m not 100% sure nodepool provides it in all cases, but regardless we can probably init it to {}.","commit_id":"39fb64a6d856af3e741c99a8b13eb712df627626"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"dea5f6176c4072e70a5d72cdca89f58eb3bbab6b","unresolved":true,"context_lines":[{"line_number":113,"context_line":"        executor_zone \u003d None"},{"line_number":114,"context_line":"        if len(nodeset.nodes):"},{"line_number":115,"context_line":"            node \u003d nodeset.getNodes()[0]"},{"line_number":116,"context_line":"            if hasattr(node, \u0027attributes\u0027) and node.attributes:"},{"line_number":117,"context_line":"                executor_zone \u003d node.attributes.get(\u0027executor-zone\u0027)"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"        zone_known \u003d False"}],"source_content_type":"text/x-python","patch_set":5,"id":"250202da_65a14925","line":116,"in_reply_to":"346a043a_7a6b3193","updated":"2021-09-01 18:27:50.000000000","message":"Looks like it is the node-attributes pool config that can be set in the nodepool config.","commit_id":"39fb64a6d856af3e741c99a8b13eb712df627626"}],"zuul/executor/server.py":[{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"86c374c4cce8553b9f5df99051fe5becd7a3aa0a","unresolved":false,"context_lines":[{"line_number":1139,"context_line":"                )"},{"line_number":1140,"context_line":"            except Exception:"},{"line_number":1141,"context_line":"                self.log.exception("},{"line_number":1142,"context_line":"                    \"Unable to return nodeset %s\", self.node_request"},{"line_number":1143,"context_line":"                )"},{"line_number":1144,"context_line":""},{"line_number":1145,"context_line":"    def _base_job_data(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"804caf6a_15361fb6","line":1142,"updated":"2021-09-02 02:28:01.000000000","message":"It was intentionally node_request -- it matches lockNodes.  But maybe we can use self.nodeset for both.","commit_id":"39fb64a6d856af3e741c99a8b13eb712df627626"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"dea5f6176c4072e70a5d72cdca89f58eb3bbab6b","unresolved":true,"context_lines":[{"line_number":1139,"context_line":"                )"},{"line_number":1140,"context_line":"            except Exception:"},{"line_number":1141,"context_line":"                self.log.exception("},{"line_number":1142,"context_line":"                    \"Unable to return nodeset %s\", self.node_request"},{"line_number":1143,"context_line":"                )"},{"line_number":1144,"context_line":""},{"line_number":1145,"context_line":"    def _base_job_data(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff4c5f39_cc52ba47","line":1142,"range":{"start_line":1142,"start_character":56,"end_line":1142,"end_character":68},"updated":"2021-09-01 18:27:50.000000000","message":"This should be self.nodeset.","commit_id":"39fb64a6d856af3e741c99a8b13eb712df627626"}],"zuul/manager/__init__.py":[{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"a190c32aca0d68f035a971af423a23e19b8ab6c0","unresolved":true,"context_lines":[{"line_number":1505,"context_line":"        # TODOv3(jeblair): handle provisioning failure here"},{"line_number":1506,"context_line":"        log \u003d get_annotated_logger(self.log, request.event_id)"},{"line_number":1507,"context_line":""},{"line_number":1508,"context_line":"        if nodeset is not None:"},{"line_number":1509,"context_line":"            build_set.jobNodeRequestComplete(request.job_name, nodeset)"},{"line_number":1510,"context_line":"        # TODO (felix): Check if the failed is still needed as the"},{"line_number":1511,"context_line":"        # NodesProvisionedEvents are now in ZooKeeper."}],"source_content_type":"text/x-python","patch_set":5,"id":"b65147ba_ce793a56","line":1508,"updated":"2021-09-01 10:13:58.000000000","message":"Is this just here as a safeguard or are there certain use-cases where the nodeset can be None?  AFAIKS the scheduler will never call this method if the nodeset is None.","commit_id":"39fb64a6d856af3e741c99a8b13eb712df627626"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"4bbd0fc02360d9663a81f086c695cd9ed01dbc8f","unresolved":true,"context_lines":[{"line_number":1505,"context_line":"        # TODOv3(jeblair): handle provisioning failure here"},{"line_number":1506,"context_line":"        log \u003d get_annotated_logger(self.log, request.event_id)"},{"line_number":1507,"context_line":""},{"line_number":1508,"context_line":"        if nodeset is not None:"},{"line_number":1509,"context_line":"            build_set.jobNodeRequestComplete(request.job_name, nodeset)"},{"line_number":1510,"context_line":"        # TODO (felix): Check if the failed is still needed as the"},{"line_number":1511,"context_line":"        # NodesProvisionedEvents are now in ZooKeeper."}],"source_content_type":"text/x-python","patch_set":5,"id":"bec5c7ee_f2b12faa","line":1508,"in_reply_to":"b65147ba_ce793a56","updated":"2021-09-01 13:01:36.000000000","message":"Seems that we need that with the change in 806816","commit_id":"39fb64a6d856af3e741c99a8b13eb712df627626"}],"zuul/model.py":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"dea5f6176c4072e70a5d72cdca89f58eb3bbab6b","unresolved":true,"context_lines":[{"line_number":886,"context_line":"        self.state_time \u003d data[\u0027state_time\u0027]"},{"line_number":887,"context_line":"        self.relative_priority \u003d data.get(\u0027relative_priority\u0027, 0)"},{"line_number":888,"context_line":"        self.event_id \u003d data[\u0027event_id\u0027]"},{"line_number":889,"context_line":"        self.nodes \u003d data.get(\u0027nodes\u0027, [])"},{"line_number":890,"context_line":""},{"line_number":891,"context_line":"    @classmethod"},{"line_number":892,"context_line":"    def fromDict(cls, data):"}],"source_content_type":"text/x-python","patch_set":5,"id":"ccba87d6_d76f0873","line":889,"updated":"2021-09-01 18:27:50.000000000","message":"Should you set labels here as well? Looks like we preset labels below with the class instantiation then call updateFromDict with the same data input. It isn\u0027t clear to me where data should be set by the class __init__ and where we want to set it in updateFromDict.","commit_id":"39fb64a6d856af3e741c99a8b13eb712df627626"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"86c374c4cce8553b9f5df99051fe5becd7a3aa0a","unresolved":false,"context_lines":[{"line_number":886,"context_line":"        self.state_time \u003d data[\u0027state_time\u0027]"},{"line_number":887,"context_line":"        self.relative_priority \u003d data.get(\u0027relative_priority\u0027, 0)"},{"line_number":888,"context_line":"        self.event_id \u003d data[\u0027event_id\u0027]"},{"line_number":889,"context_line":"        self.nodes \u003d data.get(\u0027nodes\u0027, [])"},{"line_number":890,"context_line":""},{"line_number":891,"context_line":"    @classmethod"},{"line_number":892,"context_line":"    def fromDict(cls, data):"}],"source_content_type":"text/x-python","patch_set":5,"id":"4f93efe8_5438bdf0","line":889,"updated":"2021-09-02 02:28:01.000000000","message":"labels can never change, so it doesn\u0027t need to be updated here.","commit_id":"39fb64a6d856af3e741c99a8b13eb712df627626"}],"zuul/nodepool.py":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"dea5f6176c4072e70a5d72cdca89f58eb3bbab6b","unresolved":true,"context_lines":[{"line_number":124,"context_line":"                                relative_priority, event_id)"},{"line_number":125,"context_line":"        self.requests[req.uid] \u003d req"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"        if job.nodeset.nodes:"},{"line_number":128,"context_line":"            self.zk_nodepool.submitNodeRequest("},{"line_number":129,"context_line":"                req, priority, self._updateNodeRequest)"},{"line_number":130,"context_line":"            # Logged after submission so that we have the request id"}],"source_content_type":"text/x-python","patch_set":5,"id":"fe045f81_6b54beae","line":127,"updated":"2021-09-01 18:27:50.000000000","message":"Nit: Might be good to check if labels here instead for consistency.","commit_id":"39fb64a6d856af3e741c99a8b13eb712df627626"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"86c374c4cce8553b9f5df99051fe5becd7a3aa0a","unresolved":false,"context_lines":[{"line_number":124,"context_line":"                                relative_priority, event_id)"},{"line_number":125,"context_line":"        self.requests[req.uid] \u003d req"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"        if job.nodeset.nodes:"},{"line_number":128,"context_line":"            self.zk_nodepool.submitNodeRequest("},{"line_number":129,"context_line":"                req, priority, self._updateNodeRequest)"},{"line_number":130,"context_line":"            # Logged after submission so that we have the request id"}],"source_content_type":"text/x-python","patch_set":5,"id":"5c2c8c9c_35dcf4c8","line":127,"updated":"2021-09-02 02:28:01.000000000","message":"We could, but nodeset.nodes is sort of a cantrip used in lots of places to mean \u0027a non-empty nodeset\u0027.","commit_id":"39fb64a6d856af3e741c99a8b13eb712df627626"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"dea5f6176c4072e70a5d72cdca89f58eb3bbab6b","unresolved":true,"context_lines":[{"line_number":360,"context_line":"        # re-raising the error."},{"line_number":361,"context_line":"        locked_nodes \u003d []"},{"line_number":362,"context_line":"        try:"},{"line_number":363,"context_line":"            for node_id, node in zip(request.nodes, nodeset.getNodes()):"},{"line_number":364,"context_line":"                self.zk_nodepool.updateNode(node, node_id)"},{"line_number":365,"context_line":"                if node.allocated_to !\u003d request.id:"},{"line_number":366,"context_line":"                    raise Exception(\"Node %s allocated to %s, not %s\" %"}],"source_content_type":"text/x-python","patch_set":5,"id":"c70e42fc_ba2b92e6","line":363,"range":{"start_line":363,"start_character":33,"end_line":363,"end_character":71},"updated":"2021-09-01 18:27:50.000000000","message":"Is the order guaranteed to be stable in these two lists? That is important for ensuring we don\u0027t mixup nodes of different types here.","commit_id":"39fb64a6d856af3e741c99a8b13eb712df627626"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"86c374c4cce8553b9f5df99051fe5becd7a3aa0a","unresolved":false,"context_lines":[{"line_number":360,"context_line":"        # re-raising the error."},{"line_number":361,"context_line":"        locked_nodes \u003d []"},{"line_number":362,"context_line":"        try:"},{"line_number":363,"context_line":"            for node_id, node in zip(request.nodes, nodeset.getNodes()):"},{"line_number":364,"context_line":"                self.zk_nodepool.updateNode(node, node_id)"},{"line_number":365,"context_line":"                if node.allocated_to !\u003d request.id:"},{"line_number":366,"context_line":"                    raise Exception(\"Node %s allocated to %s, not %s\" %"}],"source_content_type":"text/x-python","patch_set":5,"id":"cdc18315_5e2d42d8","line":363,"updated":"2021-09-02 02:28:01.000000000","message":"Yes, that\u0027s the nodepool protocol.","commit_id":"39fb64a6d856af3e741c99a8b13eb712df627626"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"a190c32aca0d68f035a971af423a23e19b8ab6c0","unresolved":true,"context_lines":[{"line_number":427,"context_line":"        potential use of its nodes. The nodes itself will be accepted and"},{"line_number":428,"context_line":"        locked by the executor when the corresponding job is started."},{"line_number":429,"context_line":""},{"line_number":430,"context_line":"        :returns: False if there is a problem with the request (canceled or"},{"line_number":431,"context_line":"            retrying), True if it is ready to be acted upon (success or"},{"line_number":432,"context_line":"            failure)."},{"line_number":433,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"5ec38a6f_b75fb254","line":430,"updated":"2021-09-01 10:13:58.000000000","message":"nit: return types need to be updated","commit_id":"39fb64a6d856af3e741c99a8b13eb712df627626"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"81ff14c7c0a49cf8b4036e766f38d98cbeaa262b","unresolved":true,"context_lines":[{"line_number":427,"context_line":"        potential use of its nodes. The nodes itself will be accepted and"},{"line_number":428,"context_line":"        locked by the executor when the corresponding job is started."},{"line_number":429,"context_line":""},{"line_number":430,"context_line":"        :returns: False if there is a problem with the request (canceled or"},{"line_number":431,"context_line":"            retrying), True if it is ready to be acted upon (success or"},{"line_number":432,"context_line":"            failure)."},{"line_number":433,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"21c97845_96d7db30","line":430,"in_reply_to":"5ec38a6f_b75fb254","updated":"2021-09-01 12:59:51.000000000","message":"Just noticed that this is method is refactored in 806816 anyways.","commit_id":"39fb64a6d856af3e741c99a8b13eb712df627626"}],"zuul/zk/nodepool.py":[{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"fceaa9ed1236e2d021fcef063d4301313b0a26b3","unresolved":true,"context_lines":[{"line_number":407,"context_line":"        The serialized version of the NodeRequest doesn\u0027t contain a NodeSet, so"},{"line_number":408,"context_line":"        we have to add this to the request manually. The nodeset provided to"},{"line_number":409,"context_line":"        this method will be set on the NodeRequest before updating it. This"},{"line_number":410,"context_line":"        will ensure that all nodes are updated as well."},{"line_number":411,"context_line":"        \"\"\""},{"line_number":412,"context_line":""},{"line_number":413,"context_line":"        path \u003d f\"{self.REQUEST_ROOT}/{node_request_id}\""}],"source_content_type":"text/x-python","patch_set":2,"id":"242be4ed_35274f94","line":410,"updated":"2021-08-27 09:36:54.000000000","message":"nit: doc string should to be updated.","commit_id":"1c1a27d22d9e4cb42f0f4feb4cdd23b133c44e04"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"fceaa9ed1236e2d021fcef063d4301313b0a26b3","unresolved":true,"context_lines":[{"line_number":494,"context_line":"        \"\"\""},{"line_number":495,"context_line":"        Refresh an existing node."},{"line_number":496,"context_line":""},{"line_number":497,"context_line":"        :param Node node: The node to update."},{"line_number":498,"context_line":"        \"\"\""},{"line_number":499,"context_line":"        node_path \u003d \u0027%s/%s\u0027 % (self.NODES_ROOT, node_id)"},{"line_number":500,"context_line":"        node.id \u003d node_id"}],"source_content_type":"text/x-python","patch_set":2,"id":"850f5051_f42d44df","line":497,"updated":"2021-08-27 09:36:54.000000000","message":"nit: doc string should to be updated.","commit_id":"1c1a27d22d9e4cb42f0f4feb4cdd23b133c44e04"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"86c374c4cce8553b9f5df99051fe5becd7a3aa0a","unresolved":false,"context_lines":[{"line_number":442,"context_line":"        \"\"\""},{"line_number":443,"context_line":"        node_path \u003d \u0027%s/%s\u0027 % (self.NODES_ROOT, node_id)"},{"line_number":444,"context_line":"        node.id \u003d node_id"},{"line_number":445,"context_line":"        node_data, node_stat \u003d self.kazoo_client.get(node_path)"},{"line_number":446,"context_line":"        node_data \u003d json.loads(node_data.decode(\u0027utf8\u0027))"},{"line_number":447,"context_line":"        node.updateFromDict(node_data)"},{"line_number":448,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3f823c75_0fc9225d","line":445,"updated":"2021-09-02 02:28:01.000000000","message":"I think if it failed in that manner it would be fine.","commit_id":"39fb64a6d856af3e741c99a8b13eb712df627626"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"dea5f6176c4072e70a5d72cdca89f58eb3bbab6b","unresolved":true,"context_lines":[{"line_number":442,"context_line":"        \"\"\""},{"line_number":443,"context_line":"        node_path \u003d \u0027%s/%s\u0027 % (self.NODES_ROOT, node_id)"},{"line_number":444,"context_line":"        node.id \u003d node_id"},{"line_number":445,"context_line":"        node_data, node_stat \u003d self.kazoo_client.get(node_path)"},{"line_number":446,"context_line":"        node_data \u003d json.loads(node_data.decode(\u0027utf8\u0027))"},{"line_number":447,"context_line":"        node.updateFromDict(node_data)"},{"line_number":448,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"d4b46be6_9347c837","line":445,"updated":"2021-09-01 18:27:50.000000000","message":"If the node_id doesn\u0027t match node.id is it possible that this get will fail to find json data to load? Should we guard against that?\n\nI\u0027m guessing the change of the node id would only happen to point at another different node in the database so this is probably fine.","commit_id":"39fb64a6d856af3e741c99a8b13eb712df627626"}]}