)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":35329,"name":"Per Wiklund","display_name":"Per Wiklund","email":"per.wiklund@zenseact.com","username":"elpell"},"change_message_id":"35c570e6bf65602c3769d3f43d3009f7b7c8bb9a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"9f29f959_219fc66a","updated":"2022-09-21 10:15:57.000000000","message":"recheck","commit_id":"483b2f509d8a7e9b41302d9902acebb776634a84"},{"author":{"_account_id":29671,"name":"Albin Vass","email":"opendev@albinvass.com","username":"albin_vass"},"change_message_id":"5d9b94fedfa54b9d7b3d3b620546fc3160710cba","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"b86ef66b_e3d8de2c","updated":"2022-09-23 07:00:06.000000000","message":"recheck","commit_id":"483b2f509d8a7e9b41302d9902acebb776634a84"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"bde61ce1dc61d8d47858322642d1351223b00f6f","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":10,"id":"61a7b83f_aff0e152","updated":"2022-10-06 09:17:09.000000000","message":"Some kind of documentation would be nice, maybe as an example in the static driver doc. Also a release note?","commit_id":"10f6a538b3374d414af32273e83b3d9e7772f746"},{"author":{"_account_id":35329,"name":"Per Wiklund","display_name":"Per Wiklund","email":"per.wiklund@zenseact.com","username":"elpell"},"change_message_id":"e1959ff0cc55108d45b90c40a9a221cf5927e543","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"7c2f66d5_b85c7f5c","updated":"2022-10-03 13:51:03.000000000","message":"recheck","commit_id":"10f6a538b3374d414af32273e83b3d9e7772f746"},{"author":{"_account_id":35329,"name":"Per Wiklund","display_name":"Per Wiklund","email":"per.wiklund@zenseact.com","username":"elpell"},"change_message_id":"81059da7d0c97eef272c37220e4ef76f6a4d265b","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":10,"id":"d839741f_250c1c4f","in_reply_to":"61a7b83f_aff0e152","updated":"2022-10-11 09:41:08.000000000","message":"I have added some documentation to the max-parallel-job section in the static.rst. However, I have not added a release note since the slot number is not a new configurable feature and I was unsure when a release note is required.","commit_id":"10f6a538b3374d414af32273e83b3d9e7772f746"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"66582ee0b301febd7a7cc958f9c983c667f264e8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"4b58212b_ee033bd2","in_reply_to":"d839741f_250c1c4f","updated":"2022-10-11 14:15:27.000000000","message":"Both of these are for Zuul to do.","commit_id":"10f6a538b3374d414af32273e83b3d9e7772f746"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"66582ee0b301febd7a7cc958f9c983c667f264e8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"9e4eaa0a_83e558ab","updated":"2022-10-11 14:15:27.000000000","message":"PS 13 is PS8 again, but rebased on current master (to fix a conflict).\n\nThis is the version of the change I\u0027m comfortable proposing.\n","commit_id":"08fdeed24110d278e845b2a17010f08e713f1e49"},{"author":{"_account_id":35329,"name":"Per Wiklund","display_name":"Per Wiklund","email":"per.wiklund@zenseact.com","username":"elpell"},"change_message_id":"6ab006e7b5f3de29300830d3eb8b90fd48d55be1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"a8e8fdf1_9c9f00f9","updated":"2022-10-17 06:52:01.000000000","message":"recheck","commit_id":"08fdeed24110d278e845b2a17010f08e713f1e49"},{"author":{"_account_id":35329,"name":"Per Wiklund","display_name":"Per Wiklund","email":"per.wiklund@zenseact.com","username":"elpell"},"change_message_id":"640782bd8a878dded2831da598928d4acd401475","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"ceb45880_2e0f0118","updated":"2022-10-19 07:59:27.000000000","message":"recheck","commit_id":"08fdeed24110d278e845b2a17010f08e713f1e49"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"8fd6417a64f02462ee0b3796282e3b219651995b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"8d1f5a3f_87336cbe","updated":"2022-10-19 22:20:06.000000000","message":"recheck the base images should be working now.","commit_id":"08fdeed24110d278e845b2a17010f08e713f1e49"},{"author":{"_account_id":35329,"name":"Per Wiklund","display_name":"Per Wiklund","email":"per.wiklund@zenseact.com","username":"elpell"},"change_message_id":"f191449f1a15c19cc2605f2470f0b08462f268fb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"edf3c88c_db478c84","updated":"2022-10-14 07:56:07.000000000","message":"regate","commit_id":"08fdeed24110d278e845b2a17010f08e713f1e49"}],"doc/source/static.rst":[{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"e7408f08b0f2d24849ab7e690dafc788795b1266","unresolved":true,"context_lines":[{"line_number":182,"context_line":"            :default: 1"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"            The number of jobs that can run in parallel on this node. These"},{"line_number":185,"context_line":"            nodes will share ``name``,"},{"line_number":186,"context_line":"            :attr:`providers.[static].pools.nodes.username`, and"},{"line_number":187,"context_line":"            :attr:`providers.[static].pools.nodes.connection-port`,"},{"line_number":188,"context_line":"            but have distinct ``slot numbers``. This facilitates avoiding"}],"source_content_type":"text/x-rst","patch_set":11,"id":"404d2dc7_abdebc89","line":185,"range":{"start_line":185,"start_character":12,"end_line":185,"end_character":17},"updated":"2022-10-11 12:38:27.000000000","message":"s/nodes/jobs/ ? The fact that internally multiple node objects are registered for this should be opaque to the user. Otherwise you shouldn\u0027t talk about jobs here in the first place and maybe even rename the attribute.","commit_id":"5dee91aece0a0e934a6b55743683dac8293296c6"},{"author":{"_account_id":35329,"name":"Per Wiklund","display_name":"Per Wiklund","email":"per.wiklund@zenseact.com","username":"elpell"},"change_message_id":"d841e74ff802b64b9b0f13465d367a16b40619f9","unresolved":true,"context_lines":[{"line_number":182,"context_line":"            :default: 1"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"            The number of jobs that can run in parallel on this node. These"},{"line_number":185,"context_line":"            nodes will share ``name``,"},{"line_number":186,"context_line":"            :attr:`providers.[static].pools.nodes.username`, and"},{"line_number":187,"context_line":"            :attr:`providers.[static].pools.nodes.connection-port`,"},{"line_number":188,"context_line":"            but have distinct ``slot numbers``. This facilitates avoiding"}],"source_content_type":"text/x-rst","patch_set":11,"id":"b5403b75_a8891dd0","line":185,"range":{"start_line":185,"start_character":12,"end_line":185,"end_character":17},"in_reply_to":"404d2dc7_abdebc89","updated":"2022-10-11 13:40:30.000000000","message":"You are right! Changed the formulation","commit_id":"5dee91aece0a0e934a6b55743683dac8293296c6"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"66582ee0b301febd7a7cc958f9c983c667f264e8","unresolved":false,"context_lines":[{"line_number":182,"context_line":"            :default: 1"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"            The number of jobs that can run in parallel on this node. These"},{"line_number":185,"context_line":"            nodes will share ``name``,"},{"line_number":186,"context_line":"            :attr:`providers.[static].pools.nodes.username`, and"},{"line_number":187,"context_line":"            :attr:`providers.[static].pools.nodes.connection-port`,"},{"line_number":188,"context_line":"            but have distinct ``slot numbers``. This facilitates avoiding"}],"source_content_type":"text/x-rst","patch_set":11,"id":"430c275e_09838675","line":185,"range":{"start_line":185,"start_character":12,"end_line":185,"end_character":17},"in_reply_to":"b5403b75_a8891dd0","updated":"2022-10-11 14:15:27.000000000","message":"Actually nodepool generally doesn\u0027t mention anything about jobs, so we don\u0027t need to mention this at all.  That\u0027s for the Zuul docs.","commit_id":"5dee91aece0a0e934a6b55743683dac8293296c6"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"e7408f08b0f2d24849ab7e690dafc788795b1266","unresolved":true,"context_lines":[{"line_number":183,"context_line":""},{"line_number":184,"context_line":"            The number of jobs that can run in parallel on this node. These"},{"line_number":185,"context_line":"            nodes will share ``name``,"},{"line_number":186,"context_line":"            :attr:`providers.[static].pools.nodes.username`, and"},{"line_number":187,"context_line":"            :attr:`providers.[static].pools.nodes.connection-port`,"},{"line_number":188,"context_line":"            but have distinct ``slot numbers``. This facilitates avoiding"},{"line_number":189,"context_line":"            workspace collisions on nodes with :attr: `max-parallel-jobs` \u003e1."}],"source_content_type":"text/x-rst","patch_set":11,"id":"953a50cf_9700e752","line":186,"updated":"2022-10-11 12:38:27.000000000","message":"Maybe these full references are too much, the rendered doc doesn\u0027t look very readable to me, consider replacing with just ``username`` etc.?","commit_id":"5dee91aece0a0e934a6b55743683dac8293296c6"},{"author":{"_account_id":35329,"name":"Per Wiklund","display_name":"Per Wiklund","email":"per.wiklund@zenseact.com","username":"elpell"},"change_message_id":"d841e74ff802b64b9b0f13465d367a16b40619f9","unresolved":false,"context_lines":[{"line_number":183,"context_line":""},{"line_number":184,"context_line":"            The number of jobs that can run in parallel on this node. These"},{"line_number":185,"context_line":"            nodes will share ``name``,"},{"line_number":186,"context_line":"            :attr:`providers.[static].pools.nodes.username`, and"},{"line_number":187,"context_line":"            :attr:`providers.[static].pools.nodes.connection-port`,"},{"line_number":188,"context_line":"            but have distinct ``slot numbers``. This facilitates avoiding"},{"line_number":189,"context_line":"            workspace collisions on nodes with :attr: `max-parallel-jobs` \u003e1."}],"source_content_type":"text/x-rst","patch_set":11,"id":"f912a209_1d55ee99","line":186,"in_reply_to":"953a50cf_9700e752","updated":"2022-10-11 13:40:30.000000000","message":"Done","commit_id":"5dee91aece0a0e934a6b55743683dac8293296c6"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"e7408f08b0f2d24849ab7e690dafc788795b1266","unresolved":true,"context_lines":[{"line_number":185,"context_line":"            nodes will share ``name``,"},{"line_number":186,"context_line":"            :attr:`providers.[static].pools.nodes.username`, and"},{"line_number":187,"context_line":"            :attr:`providers.[static].pools.nodes.connection-port`,"},{"line_number":188,"context_line":"            but have distinct ``slot numbers``. This facilitates avoiding"},{"line_number":189,"context_line":"            workspace collisions on nodes with :attr: `max-parallel-jobs` \u003e1."}],"source_content_type":"text/x-rst","patch_set":11,"id":"3edb3f03_5ba01f27","line":188,"range":{"start_line":188,"start_character":30,"end_line":188,"end_character":46},"updated":"2022-10-11 12:38:27.000000000","message":"I would rephrase this as\n\n``slot`` numbers\n\nto visualize how the node attribute will be named.","commit_id":"5dee91aece0a0e934a6b55743683dac8293296c6"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"66582ee0b301febd7a7cc958f9c983c667f264e8","unresolved":false,"context_lines":[{"line_number":185,"context_line":"            nodes will share ``name``,"},{"line_number":186,"context_line":"            :attr:`providers.[static].pools.nodes.username`, and"},{"line_number":187,"context_line":"            :attr:`providers.[static].pools.nodes.connection-port`,"},{"line_number":188,"context_line":"            but have distinct ``slot numbers``. This facilitates avoiding"},{"line_number":189,"context_line":"            workspace collisions on nodes with :attr: `max-parallel-jobs` \u003e1."}],"source_content_type":"text/x-rst","patch_set":11,"id":"5c29b152_9dc7fce7","line":188,"range":{"start_line":188,"start_character":30,"end_line":188,"end_character":46},"in_reply_to":"3edb3f03_5ba01f27","updated":"2022-10-11 14:15:27.000000000","message":"Again, this is for zuul docs.","commit_id":"5dee91aece0a0e934a6b55743683dac8293296c6"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"e7408f08b0f2d24849ab7e690dafc788795b1266","unresolved":true,"context_lines":[{"line_number":186,"context_line":"            :attr:`providers.[static].pools.nodes.username`, and"},{"line_number":187,"context_line":"            :attr:`providers.[static].pools.nodes.connection-port`,"},{"line_number":188,"context_line":"            but have distinct ``slot numbers``. This facilitates avoiding"},{"line_number":189,"context_line":"            workspace collisions on nodes with :attr: `max-parallel-jobs` \u003e1."}],"source_content_type":"text/x-rst","patch_set":11,"id":"2edda001_b103ef3a","line":189,"range":{"start_line":189,"start_character":53,"end_line":189,"end_character":54},"updated":"2022-10-11 12:38:27.000000000","message":"The extra space breaks formatting.","commit_id":"5dee91aece0a0e934a6b55743683dac8293296c6"},{"author":{"_account_id":35329,"name":"Per Wiklund","display_name":"Per Wiklund","email":"per.wiklund@zenseact.com","username":"elpell"},"change_message_id":"d841e74ff802b64b9b0f13465d367a16b40619f9","unresolved":false,"context_lines":[{"line_number":186,"context_line":"            :attr:`providers.[static].pools.nodes.username`, and"},{"line_number":187,"context_line":"            :attr:`providers.[static].pools.nodes.connection-port`,"},{"line_number":188,"context_line":"            but have distinct ``slot numbers``. This facilitates avoiding"},{"line_number":189,"context_line":"            workspace collisions on nodes with :attr: `max-parallel-jobs` \u003e1."}],"source_content_type":"text/x-rst","patch_set":11,"id":"f8ceccc7_b9a211b3","line":189,"range":{"start_line":189,"start_character":53,"end_line":189,"end_character":54},"in_reply_to":"2edda001_b103ef3a","updated":"2022-10-11 13:40:30.000000000","message":"Done","commit_id":"5dee91aece0a0e934a6b55743683dac8293296c6"}],"nodepool/driver/static/provider.py":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"99c65e3d99e302745c039708d11612dbe5d3b9de","unresolved":true,"context_lines":[{"line_number":377,"context_line":"                # This case covers an existing node, but with a decreased"},{"line_number":378,"context_line":"                # max-parallel-jobs value."},{"line_number":379,"context_line":"                try:"},{"line_number":380,"context_line":"                    self.deregisterNode(node)"},{"line_number":381,"context_line":"                except Exception:"},{"line_number":382,"context_line":"                    self.log.exception(\"Couldn\u0027t deregister static node:\")"},{"line_number":383,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"26017d53_b3d925ba","line":380,"updated":"2022-02-23 19:02:19.000000000","message":"Do we also need to remove the node from the _node_slits[nodeTuple(static_node)] list to avoid reprocessing it over and over again?","commit_id":"6221ac6e572c8a65bd1000fddb00a7138d64ab54"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"1d3316c9971f5256812e9df1660a091aa0ae4f5b","unresolved":false,"context_lines":[{"line_number":377,"context_line":"                # This case covers an existing node, but with a decreased"},{"line_number":378,"context_line":"                # max-parallel-jobs value."},{"line_number":379,"context_line":"                try:"},{"line_number":380,"context_line":"                    self.deregisterNode(node)"},{"line_number":381,"context_line":"                except Exception:"},{"line_number":382,"context_line":"                    self.log.exception(\"Couldn\u0027t deregister static node:\")"},{"line_number":383,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"bb4ca701_0465c36e","line":380,"in_reply_to":"26017d53_b3d925ba","updated":"2022-09-19 23:39:47.000000000","message":"In both cases where this method is called, it\u0027s shortly after getRegisteredNodes is called, so the _node_slots list will have been updated.  It should either contain the correct data, or possibly some extra slots from a max-parallel-jobs reduction which hasn\u0027t been able to be contracted yet for some reason (possibly the node was locked when the reduction happened).  If we get to this point with an extra slot, we will try to deregister the ZK node, then if that succeeds, the next time we call getRegisteredNodes the _node_slots will be updated and will no longer have the extra slots.\n\nPut another way, the sequence is this:\n1. Update node_slots to represent the union of config and zookeeper\n2. Attempt to remove extra nodes in node_slots from zookeeper\n\nAnd we always call the method that does step 1 (getRegisteredNodes) before step 2 (syncNodeCount or _start).","commit_id":"6221ac6e572c8a65bd1000fddb00a7138d64ab54"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"99c65e3d99e302745c039708d11612dbe5d3b9de","unresolved":true,"context_lines":[{"line_number":424,"context_line":"            if node_tuple not in static_nodes:"},{"line_number":425,"context_line":"                for node in nodes:"},{"line_number":426,"context_line":"                    try:"},{"line_number":427,"context_line":"                        self.deregisterNode(node)"},{"line_number":428,"context_line":"                    except Exception:"},{"line_number":429,"context_line":"                        self.log.exception(\"Couldn\u0027t deregister static node:\")"},{"line_number":430,"context_line":"                        continue"}],"source_content_type":"text/x-python","patch_set":5,"id":"eda2bf5d_88308d67","line":427,"updated":"2022-02-23 19:02:19.000000000","message":"Similar question as above. I think this may end up retriggering over and over again since deregisterNode(node) only updates the zk record?","commit_id":"6221ac6e572c8a65bd1000fddb00a7138d64ab54"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"1d3316c9971f5256812e9df1660a091aa0ae4f5b","unresolved":false,"context_lines":[{"line_number":424,"context_line":"            if node_tuple not in static_nodes:"},{"line_number":425,"context_line":"                for node in nodes:"},{"line_number":426,"context_line":"                    try:"},{"line_number":427,"context_line":"                        self.deregisterNode(node)"},{"line_number":428,"context_line":"                    except Exception:"},{"line_number":429,"context_line":"                        self.log.exception(\"Couldn\u0027t deregister static node:\")"},{"line_number":430,"context_line":"                        continue"}],"source_content_type":"text/x-python","patch_set":5,"id":"efebe4ee_1a6b4cb1","line":427,"in_reply_to":"eda2bf5d_88308d67","updated":"2022-09-19 23:39:47.000000000","message":"The same is true as above.","commit_id":"6221ac6e572c8a65bd1000fddb00a7138d64ab54"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"bde61ce1dc61d8d47858322642d1351223b00f6f","unresolved":true,"context_lines":[{"line_number":260,"context_line":"            static_node[\"username\"],"},{"line_number":261,"context_line":"            static_node[\"connection-port\"],"},{"line_number":262,"context_line":"            static_node[\"shell-type\"],"},{"line_number":263,"context_line":"            static_node[\"connection-type\"],"},{"line_number":264,"context_line":"            static_node[\"python-path\"],"},{"line_number":265,"context_line":"            host_keys,"},{"line_number":266,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":10,"id":"6a65890f_079d7966","line":263,"updated":"2022-10-06 09:17:09.000000000","message":"This seems unrelated, bad rebase?","commit_id":"10f6a538b3374d414af32273e83b3d9e7772f746"},{"author":{"_account_id":35329,"name":"Per Wiklund","display_name":"Per Wiklund","email":"per.wiklund@zenseact.com","username":"elpell"},"change_message_id":"81059da7d0c97eef272c37220e4ef76f6a4d265b","unresolved":false,"context_lines":[{"line_number":260,"context_line":"            static_node[\"username\"],"},{"line_number":261,"context_line":"            static_node[\"connection-port\"],"},{"line_number":262,"context_line":"            static_node[\"shell-type\"],"},{"line_number":263,"context_line":"            static_node[\"connection-type\"],"},{"line_number":264,"context_line":"            static_node[\"python-path\"],"},{"line_number":265,"context_line":"            host_keys,"},{"line_number":266,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":10,"id":"1ee50f33_9a65cea6","line":263,"in_reply_to":"6a65890f_079d7966","updated":"2022-10-11 09:41:08.000000000","message":"Ack","commit_id":"10f6a538b3374d414af32273e83b3d9e7772f746"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"e7408f08b0f2d24849ab7e690dafc788795b1266","unresolved":true,"context_lines":[{"line_number":17,"context_line":"import math"},{"line_number":18,"context_line":"import threading"},{"line_number":19,"context_line":"from concurrent.futures.thread import ThreadPoolExecutor"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"from collections import Counter, namedtuple"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"from nodepool import exceptions"}],"source_content_type":"text/x-python","patch_set":11,"id":"2bde1d34_8b858cf3","side":"PARENT","line":20,"updated":"2022-10-11 12:38:27.000000000","message":"The empty line should stay IMO, it separates library modules from external ones.","commit_id":"19d7c65b0e57a9a95177ef16db3125036540089d"},{"author":{"_account_id":35329,"name":"Per Wiklund","display_name":"Per Wiklund","email":"per.wiklund@zenseact.com","username":"elpell"},"change_message_id":"d841e74ff802b64b9b0f13465d367a16b40619f9","unresolved":true,"context_lines":[{"line_number":17,"context_line":"import math"},{"line_number":18,"context_line":"import threading"},{"line_number":19,"context_line":"from concurrent.futures.thread import ThreadPoolExecutor"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"from collections import Counter, namedtuple"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"from nodepool import exceptions"}],"source_content_type":"text/x-python","patch_set":11,"id":"6f911249_2fa63fa9","side":"PARENT","line":20,"in_reply_to":"2bde1d34_8b858cf3","updated":"2022-10-11 13:40:30.000000000","message":"The empty line separated concurrent.futures.thread and collections, and aren\u0027t both of those external libraries? Making the empty line is not necessary according to me","commit_id":"19d7c65b0e57a9a95177ef16db3125036540089d"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"66582ee0b301febd7a7cc958f9c983c667f264e8","unresolved":false,"context_lines":[{"line_number":17,"context_line":"import math"},{"line_number":18,"context_line":"import threading"},{"line_number":19,"context_line":"from concurrent.futures.thread import ThreadPoolExecutor"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"from collections import Counter, namedtuple"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"from nodepool import exceptions"}],"source_content_type":"text/x-python","patch_set":11,"id":"ca08f677_00b1bee7","side":"PARENT","line":20,"in_reply_to":"6f911249_2fa63fa9","updated":"2022-10-11 14:15:27.000000000","message":"We generally try to separate imports as described, but we also try to avoid changing things that don\u0027t need changing, and we try to avoid commenting on whitespace changes in reviews.  They should almost never be considered a -1.\n\nAt any rate, collections is in the standard library, so this change is fine.","commit_id":"19d7c65b0e57a9a95177ef16db3125036540089d"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"e7408f08b0f2d24849ab7e690dafc788795b1266","unresolved":true,"context_lines":[{"line_number":58,"context_line":"    def _getSlot(self, node):"},{"line_number":59,"context_line":"        return self._node_slots[nodeTuple(node)].index(node)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def checkHost(self, static_node):"},{"line_number":62,"context_line":"        \u0027\u0027\u0027Check node is reachable\u0027\u0027\u0027"},{"line_number":63,"context_line":"        # only gather host keys if the connection type is ssh or network_cli"},{"line_number":64,"context_line":"        gather_hostkeys \u003d ("}],"source_content_type":"text/x-python","patch_set":11,"id":"afb7452c_9211e937","line":61,"range":{"start_line":61,"start_character":24,"end_line":61,"end_character":35},"updated":"2022-10-11 12:38:27.000000000","message":"Renaming the argument seems unrelated and makes it more difficult to see what this patch is actually doing. If you think that the rename is necessary, I would prefer to see it split out into a dedicated patch.","commit_id":"5dee91aece0a0e934a6b55743683dac8293296c6"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"66582ee0b301febd7a7cc958f9c983c667f264e8","unresolved":false,"context_lines":[{"line_number":58,"context_line":"    def _getSlot(self, node):"},{"line_number":59,"context_line":"        return self._node_slots[nodeTuple(node)].index(node)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def checkHost(self, static_node):"},{"line_number":62,"context_line":"        \u0027\u0027\u0027Check node is reachable\u0027\u0027\u0027"},{"line_number":63,"context_line":"        # only gather host keys if the connection type is ssh or network_cli"},{"line_number":64,"context_line":"        gather_hostkeys \u003d ("}],"source_content_type":"text/x-python","patch_set":11,"id":"12e87772_c644a5aa","line":61,"range":{"start_line":61,"start_character":24,"end_line":61,"end_character":35},"in_reply_to":"4a3be32d_88a63b41","updated":"2022-10-11 14:15:27.000000000","message":"The name \"static_node\" and \"node\" have important distinctions.  This helps the developer understand that we\u0027re dealing with a config object and not a zk object.  This should stay in the change.","commit_id":"5dee91aece0a0e934a6b55743683dac8293296c6"},{"author":{"_account_id":35329,"name":"Per Wiklund","display_name":"Per Wiklund","email":"per.wiklund@zenseact.com","username":"elpell"},"change_message_id":"d841e74ff802b64b9b0f13465d367a16b40619f9","unresolved":true,"context_lines":[{"line_number":58,"context_line":"    def _getSlot(self, node):"},{"line_number":59,"context_line":"        return self._node_slots[nodeTuple(node)].index(node)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def checkHost(self, static_node):"},{"line_number":62,"context_line":"        \u0027\u0027\u0027Check node is reachable\u0027\u0027\u0027"},{"line_number":63,"context_line":"        # only gather host keys if the connection type is ssh or network_cli"},{"line_number":64,"context_line":"        gather_hostkeys \u003d ("}],"source_content_type":"text/x-python","patch_set":11,"id":"4a3be32d_88a63b41","line":61,"range":{"start_line":61,"start_character":24,"end_line":61,"end_character":35},"in_reply_to":"afb7452c_9211e937","updated":"2022-10-11 13:40:30.000000000","message":"static_node seems to be a common name throughout nodepool/driver/static/provider.py, because of this i believe it makes sense to rename it here as well. However, I can agree that it is not directly related to this change\u0027s topic.","commit_id":"5dee91aece0a0e934a6b55743683dac8293296c6"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"031627e71dc6c6e953963facebfdb5d75432e7b5","unresolved":true,"context_lines":[{"line_number":199,"context_line":"                # the node, it\u0027s just that the node metadata doesn\u0027t"},{"line_number":200,"context_line":"                # have a slot number."},{"line_number":201,"context_line":"                idx \u003d node_slots[nodeTuple(node)].index(None)"},{"line_number":202,"context_line":"                node_slots[nodeTuple(node)][idx] \u003d node"},{"line_number":203,"context_line":"            else:"},{"line_number":204,"context_line":"                # We have more nodes than expected."},{"line_number":205,"context_line":"                self.log.warning(\"Tracking excess node %s as %s slot %s\","}],"source_content_type":"text/x-python","patch_set":13,"id":"42e1eb79_be56695f","line":202,"updated":"2022-10-11 22:09:23.000000000","message":"Do we need to set node.slot to idx here? In particular entries to unslotted_nodes may have node.slot set to None accordingto the else at line 179. Additionally, if a slot shifts somehow we\u0027d need to reconcile that?","commit_id":"08fdeed24110d278e845b2a17010f08e713f1e49"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"7c6552c1e118d805222f7ab3b477d3549acad562","unresolved":true,"context_lines":[{"line_number":199,"context_line":"                # the node, it\u0027s just that the node metadata doesn\u0027t"},{"line_number":200,"context_line":"                # have a slot number."},{"line_number":201,"context_line":"                idx \u003d node_slots[nodeTuple(node)].index(None)"},{"line_number":202,"context_line":"                node_slots[nodeTuple(node)][idx] \u003d node"},{"line_number":203,"context_line":"            else:"},{"line_number":204,"context_line":"                # We have more nodes than expected."},{"line_number":205,"context_line":"                self.log.warning(\"Tracking excess node %s as %s slot %s\","}],"source_content_type":"text/x-python","patch_set":13,"id":"73b05efa_0f53ab3a","line":202,"in_reply_to":"42e1eb79_be56695f","updated":"2022-10-11 22:20:07.000000000","message":"That is an option, but I don\u0027t think we need to do that.  Node.slot\u003d\u003dNone should only happen in the case where we upgrade, so the actual slot number of the node isn\u0027t that important.  The next time the node is (re)created, it will get a slot number assigned to it, and things will be good from there.  Actually assigning the slot here would be fairly complex because we would need to lock the node, and it may already be locked.  Instead, the approach is to just have it occupy any slot in memory until it\u0027s gone.  From the consumer\u0027s (zuul\u0027s) pov, it wasn\u0027t created with a slot, so it will just be None.","commit_id":"08fdeed24110d278e845b2a17010f08e713f1e49"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"031627e71dc6c6e953963facebfdb5d75432e7b5","unresolved":true,"context_lines":[{"line_number":205,"context_line":"                self.log.warning(\"Tracking excess node %s as %s slot %s\","},{"line_number":206,"context_line":"                                 node, nodeTuple(node),"},{"line_number":207,"context_line":"                                 len(node_slots[nodeTuple(node)]))"},{"line_number":208,"context_line":"                node_slots[nodeTuple(node)].append(node)"},{"line_number":209,"context_line":"        self._node_slots \u003d node_slots"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    def registerNodeFromConfig(self, provider_name, pool, static_node,"}],"source_content_type":"text/x-python","patch_set":13,"id":"bc91ed39_369410c6","line":208,"updated":"2022-10-11 22:09:23.000000000","message":"See above though I think it would be node.slot \u003d len(node_slots[nodeTuple(node)]) - 1","commit_id":"08fdeed24110d278e845b2a17010f08e713f1e49"}],"nodepool/tests/unit/test_driver_static.py":[{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"bde61ce1dc61d8d47858322642d1351223b00f6f","unresolved":true,"context_lines":[{"line_number":294,"context_line":"        nodes \u003d self.waitForNodes(\u0027fake-label\u0027)"},{"line_number":295,"context_line":"        self.assertIn(\u0027fake-label\u0027, nodes[0].type)"},{"line_number":296,"context_line":"        self.assertIn(\u0027fake-label2\u0027, nodes[0].type)"},{"line_number":297,"context_line":"        self.assertEqual(nodes[0].slot, 0)"},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"    def test_static_handler(self):"},{"line_number":300,"context_line":"        configfile \u003d self.setup_config(\u0027static.yaml\u0027)"}],"source_content_type":"text/x-python","patch_set":10,"id":"7e806bd6_9cf94486","line":297,"updated":"2022-10-06 09:17:09.000000000","message":"Is it really necessary to add this assert almost everywhere? We don\u0027t check most other attributes either, so I\u0027m not sure what should be so special here to warrant this.","commit_id":"10f6a538b3374d414af32273e83b3d9e7772f746"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"e7408f08b0f2d24849ab7e690dafc788795b1266","unresolved":false,"context_lines":[{"line_number":294,"context_line":"        nodes \u003d self.waitForNodes(\u0027fake-label\u0027)"},{"line_number":295,"context_line":"        self.assertIn(\u0027fake-label\u0027, nodes[0].type)"},{"line_number":296,"context_line":"        self.assertIn(\u0027fake-label2\u0027, nodes[0].type)"},{"line_number":297,"context_line":"        self.assertEqual(nodes[0].slot, 0)"},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"    def test_static_handler(self):"},{"line_number":300,"context_line":"        configfile \u003d self.setup_config(\u0027static.yaml\u0027)"}],"source_content_type":"text/x-python","patch_set":10,"id":"6620f4a9_facbe64d","line":297,"in_reply_to":"71564d86_fac4ddb6","updated":"2022-10-11 12:38:27.000000000","message":"Ack","commit_id":"10f6a538b3374d414af32273e83b3d9e7772f746"},{"author":{"_account_id":35329,"name":"Per Wiklund","display_name":"Per Wiklund","email":"per.wiklund@zenseact.com","username":"elpell"},"change_message_id":"81059da7d0c97eef272c37220e4ef76f6a4d265b","unresolved":true,"context_lines":[{"line_number":294,"context_line":"        nodes \u003d self.waitForNodes(\u0027fake-label\u0027)"},{"line_number":295,"context_line":"        self.assertIn(\u0027fake-label\u0027, nodes[0].type)"},{"line_number":296,"context_line":"        self.assertIn(\u0027fake-label2\u0027, nodes[0].type)"},{"line_number":297,"context_line":"        self.assertEqual(nodes[0].slot, 0)"},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"    def test_static_handler(self):"},{"line_number":300,"context_line":"        configfile \u003d self.setup_config(\u0027static.yaml\u0027)"}],"source_content_type":"text/x-python","patch_set":10,"id":"71564d86_fac4ddb6","line":297,"in_reply_to":"7e806bd6_9cf94486","updated":"2022-10-11 09:41:08.000000000","message":"The slot assert seemed overly used. I have reduced it to be included where other attributes are also checked and in test_static_parallel_decrease since this feature is affecting the slot numbers.","commit_id":"10f6a538b3374d414af32273e83b3d9e7772f746"}],"nodepool/zk/zookeeper.py":[{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"bde61ce1dc61d8d47858322642d1351223b00f6f","unresolved":true,"context_lines":[{"line_number":689,"context_line":"        else:"},{"line_number":690,"context_line":"            self.hold_expiration \u003d hold_expiration"},{"line_number":691,"context_line":"        self.resources \u003d d.get(\u0027resources\u0027)"},{"line_number":692,"context_line":"        self.slot \u003d d.get(\u0027slot\u0027)"},{"line_number":693,"context_line":"        self.attributes \u003d d.get(\u0027attributes\u0027)"},{"line_number":694,"context_line":"        self.python_path \u003d d.get(\u0027python_path\u0027)"},{"line_number":695,"context_line":"        self.shell_type \u003d d.get(\u0027shell_type\u0027)"}],"source_content_type":"text/x-python","patch_set":10,"id":"ac97ecf5_aa35d7df","line":692,"updated":"2022-10-06 09:17:09.000000000","message":"Does this need a fallback in order not to break on updates?","commit_id":"10f6a538b3374d414af32273e83b3d9e7772f746"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"e7408f08b0f2d24849ab7e690dafc788795b1266","unresolved":false,"context_lines":[{"line_number":689,"context_line":"        else:"},{"line_number":690,"context_line":"            self.hold_expiration \u003d hold_expiration"},{"line_number":691,"context_line":"        self.resources \u003d d.get(\u0027resources\u0027)"},{"line_number":692,"context_line":"        self.slot \u003d d.get(\u0027slot\u0027)"},{"line_number":693,"context_line":"        self.attributes \u003d d.get(\u0027attributes\u0027)"},{"line_number":694,"context_line":"        self.python_path \u003d d.get(\u0027python_path\u0027)"},{"line_number":695,"context_line":"        self.shell_type \u003d d.get(\u0027shell_type\u0027)"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f0f52b9_44e8d03d","line":692,"in_reply_to":"797b9bc8_8c909a12","updated":"2022-10-11 12:38:27.000000000","message":"Ack","commit_id":"10f6a538b3374d414af32273e83b3d9e7772f746"},{"author":{"_account_id":35329,"name":"Per Wiklund","display_name":"Per Wiklund","email":"per.wiklund@zenseact.com","username":"elpell"},"change_message_id":"81059da7d0c97eef272c37220e4ef76f6a4d265b","unresolved":true,"context_lines":[{"line_number":689,"context_line":"        else:"},{"line_number":690,"context_line":"            self.hold_expiration \u003d hold_expiration"},{"line_number":691,"context_line":"        self.resources \u003d d.get(\u0027resources\u0027)"},{"line_number":692,"context_line":"        self.slot \u003d d.get(\u0027slot\u0027)"},{"line_number":693,"context_line":"        self.attributes \u003d d.get(\u0027attributes\u0027)"},{"line_number":694,"context_line":"        self.python_path \u003d d.get(\u0027python_path\u0027)"},{"line_number":695,"context_line":"        self.shell_type \u003d d.get(\u0027shell_type\u0027)"}],"source_content_type":"text/x-python","patch_set":10,"id":"797b9bc8_8c909a12","line":692,"in_reply_to":"ac97ecf5_aa35d7df","updated":"2022-10-11 09:41:08.000000000","message":"I have added a fallback to slot 0 to prevent returning none during upgrades","commit_id":"10f6a538b3374d414af32273e83b3d9e7772f746"}]}