)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"6a2455116422a34b988b0c85c22d70b819ccc36b","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"3bd43ff3_48540bc4","updated":"2023-12-18 23:48:40.000000000","message":"Please provide reviews and feedback, however, this should not merge until after I\u0027ve completed work on the Ironic side to improve tempest testing. That being said; I\u0027m extremely happy to make this change better while I\u0027m doing that, so review away :D","commit_id":"89dcf56d5de69d80aa109e7b79238a6f17029502"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"7109173fecf102b502215d3bc4d583037367b9dc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"de824944_80502aac","updated":"2024-02-25 21:13:39.000000000","message":"sending old messages that were left in drafts","commit_id":"5adaea90ed8259b0f5336212759cd543cd1ed65e"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0eb0bdb0f456e95df2bc665d8ec69288f7db7030","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"e2fb0260_5dbd92cf","updated":"2024-02-25 12:00:35.000000000","message":"stills has one line lenght issue but otherwise i think this is fine to proceed with","commit_id":"5adaea90ed8259b0f5336212759cd543cd1ed65e"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"00f9dc16f13cb6e20cfbf43b9cb3cc749445705e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"c4d00d01_cc2943d1","updated":"2024-02-28 11:18:09.000000000","message":"Ergh, https://zuul.opendev.org/t/openstack/build/cf4ecb8750004d47a759719ef886f41a was not working 😞\nLooks to me it wasn\u0027t related to this series so I rechecked it : \n\n2024-02-27 22:02:22.673592 | controller | ++ /opt/stack/ironic/devstack/lib/ironic:downgrade_dnsmasq:3443 :   wget http://archive.ubuntu.com/ubuntu/pool/main/d/dnsmasq/dnsmasq-base_2.80-1.1ubuntu1.7_amd64.deb\n2024-02-27 22:02:22.676658 | controller | --2024-02-27 22:02:22--  http://archive.ubuntu.com/ubuntu/pool/main/d/dnsmasq/dnsmasq-base_2.80-1.1ubuntu1.7_amd64.deb\n2024-02-27 22:02:22.691599 | controller | Resolving archive.ubuntu.com (archive.ubuntu.com)... 2620:2d:4002:1::103, 2620:2d:4000:1::16, 2620:2d:4000:1::19, ...\n2024-02-27 22:02:22.710370 | controller | Connecting to archive.ubuntu.com (archive.ubuntu.com)|2620:2d:4002:1::103|:80... connected.\n2024-02-27 22:02:22.728333 | controller | HTTP request sent, awaiting response... 404 Not Found\n2024-02-27 22:02:22.731678 | controller | 2024-02-27 22:02:22 ERROR 404: Not Found.","commit_id":"f1a4857d612cff0ecca1be3e914e57e9db6ac275"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"9d1580d2b4caa3d31fe8f7b832393f32ab876263","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"4027f7e4_1a0164ff","updated":"2024-02-27 09:46:59.000000000","message":"I\u0027m quite good with this but do we have now a Tempest test checking this series ?\nI don\u0027t see any zuul file modification by the series.","commit_id":"f1a4857d612cff0ecca1be3e914e57e9db6ac275"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"696a726a5346fd093b55505776220dbd092a9ab6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"c5dbcc78_dd96e5b6","updated":"2024-02-29 16:13:47.000000000","message":"Okay, eventually I\u0027m OK with merging it today before FeatureFreeze. This is a bit sad that we had the Tempest test a bit late but OK.","commit_id":"f1a4857d612cff0ecca1be3e914e57e9db6ac275"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"536e33d055bdca73682074d7c6ed004eeb936bed","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"00727050_2c8878f2","updated":"2024-02-28 11:18:35.000000000","message":"Reproviding my +1.","commit_id":"f1a4857d612cff0ecca1be3e914e57e9db6ac275"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"3a5f1d9fbefd52cc24530384e17b727cfb2b3d1d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"5a56ef56_d08902f3","in_reply_to":"4027f7e4_1a0164ff","updated":"2024-02-27 15:38:36.000000000","message":"New sharding related tests since last cycle:\n- https://opendev.org/openstack/ironic-tempest-plugin/commit/f2b9c74c9cb8459a979d9002aae3c1a41737c77a Ironic API testing for sharding\n\nWIP tests:\nAttempts by both Sean and I to get working sharding in CI (only one of the zuul jobs will merge, most likely)\n- https://review.opendev.org/c/openstack/ironic/+/894460\n- https://review.opendev.org/c/openstack/nova/+/910333\n\nI\u0027ve tested this extensively locally and this cycle we have completely eradicated the old ironicclient code in the driver -- which was the root cause of all the confusion last cycle. (tl;dr: we were checking with ironic *client* for microversion support then using SDK to make the API call; now everything uses SDK).","commit_id":"f1a4857d612cff0ecca1be3e914e57e9db6ac275"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"00f9dc16f13cb6e20cfbf43b9cb3cc749445705e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"5f603b66_a74a3d62","in_reply_to":"5a56ef56_d08902f3","updated":"2024-02-28 11:18:09.000000000","message":"Ack, thanks.","commit_id":"f1a4857d612cff0ecca1be3e914e57e9db6ac275"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"b548ac21abbb9a9ae46379939b63620ceb679373","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"9c95e9d7_dc37793c","in_reply_to":"c4d00d01_cc2943d1","updated":"2024-02-28 16:32:04.000000000","message":"Yeah, that broke when ubuntu pulled the old package from their mirrors; we fixed in https://review.opendev.org/c/openstack/ironic/+/888121","commit_id":"f1a4857d612cff0ecca1be3e914e57e9db6ac275"}],"nova/conf/ironic.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"b8dcbd8aa29f300325137fad40cb9a8d40b3dddd","unresolved":true,"context_lines":[{"line_number":90,"context_line":"        help\u003d\u0027Specify which ironic shard this nova-compute will manage. \u0027"},{"line_number":91,"context_line":"             \u0027This allows you to shard Ironic nodes between compute \u0027"},{"line_number":92,"context_line":"             \u0027services across conductors and conductor groups. \u0027"},{"line_number":93,"context_line":"             \u0027When a shard is set, the peer_list configuraton is ignored. \u0027"},{"line_number":94,"context_line":"             \u0027We require that there is at most one nova-compute service \u0027"},{"line_number":95,"context_line":"             \u0027for each shard.\u0027),"},{"line_number":96,"context_line":"    cfg.ListOpt("}],"source_content_type":"text/x-python","patch_set":2,"id":"8ef71a42_e6ff50e3","line":93,"range":{"start_line":93,"start_character":14,"end_line":93,"end_character":74},"updated":"2024-01-08 15:04:14.000000000","message":"this is the pep8 failure\n\nnova/conf/ironic.py:93: configuraton \u003d\u003d\u003e configuration","commit_id":"5498fef8f65e53c466530382c7ed0d14588cba31"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0eb0bdb0f456e95df2bc665d8ec69288f7db7030","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        help\u003d\u0027Specify which ironic shard this nova-compute will manage. \u0027"},{"line_number":91,"context_line":"             \u0027This allows you to shard Ironic nodes between compute \u0027"},{"line_number":92,"context_line":"             \u0027services across conductors and conductor groups. \u0027"},{"line_number":93,"context_line":"             \u0027When a shard is set, the peer_list configuraton is ignored. \u0027"},{"line_number":94,"context_line":"             \u0027We require that there is at most one nova-compute service \u0027"},{"line_number":95,"context_line":"             \u0027for each shard.\u0027),"},{"line_number":96,"context_line":"    cfg.ListOpt("}],"source_content_type":"text/x-python","patch_set":2,"id":"56133f4a_d57e7330","line":93,"range":{"start_line":93,"start_character":14,"end_line":93,"end_character":74},"in_reply_to":"8ef71a42_e6ff50e3","updated":"2024-02-25 12:00:35.000000000","message":"Done","commit_id":"5498fef8f65e53c466530382c7ed0d14588cba31"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"7109173fecf102b502215d3bc4d583037367b9dc","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        help\u003d\u0027Specify which ironic shard this nova-compute will manage. \u0027"},{"line_number":91,"context_line":"             \u0027This allows you to shard Ironic nodes between compute \u0027"},{"line_number":92,"context_line":"             \u0027services across conductors and conductor groups. \u0027"},{"line_number":93,"context_line":"             \u0027When a shard is set, the peer_list configuraton is ignored. \u0027"},{"line_number":94,"context_line":"             \u0027We require that there is at most one nova-compute service \u0027"},{"line_number":95,"context_line":"             \u0027for each shard.\u0027),"},{"line_number":96,"context_line":"    cfg.ListOpt("}],"source_content_type":"text/x-python","patch_set":2,"id":"c83c0f61_47aafc34","line":93,"range":{"start_line":93,"start_character":14,"end_line":93,"end_character":74},"in_reply_to":"8ef71a42_e6ff50e3","updated":"2024-02-25 21:13:39.000000000","message":"Done","commit_id":"5498fef8f65e53c466530382c7ed0d14588cba31"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0eb0bdb0f456e95df2bc665d8ec69288f7db7030","unresolved":true,"context_lines":[{"line_number":86,"context_line":"        \u0027shard\u0027,"},{"line_number":87,"context_line":"        default\u003dNone,"},{"line_number":88,"context_line":"        max_length\u003d255,"},{"line_number":89,"context_line":"        regex\u003dr\u0027^[a-zA-Z0-9_.-]*$\u0027,"},{"line_number":90,"context_line":"        help\u003d\u0027Specify which ironic shard this nova-compute will manage. \u0027"},{"line_number":91,"context_line":"             \u0027This allows you to shard Ironic nodes between compute \u0027"},{"line_number":92,"context_line":"             \u0027services across conductors and conductor groups. \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"d9bfa582_17dee1fc","line":89,"range":{"start_line":89,"start_character":14,"end_line":89,"end_character":33},"updated":"2024-02-25 12:00:35.000000000","message":"r’^[a-zA-Z0-9_.-]+$’\n\nthis may be more correct but its not rquired","commit_id":"5adaea90ed8259b0f5336212759cd543cd1ed65e"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"3a5f1d9fbefd52cc24530384e17b727cfb2b3d1d","unresolved":false,"context_lines":[{"line_number":86,"context_line":"        \u0027shard\u0027,"},{"line_number":87,"context_line":"        default\u003dNone,"},{"line_number":88,"context_line":"        max_length\u003d255,"},{"line_number":89,"context_line":"        regex\u003dr\u0027^[a-zA-Z0-9_.-]*$\u0027,"},{"line_number":90,"context_line":"        help\u003d\u0027Specify which ironic shard this nova-compute will manage. \u0027"},{"line_number":91,"context_line":"             \u0027This allows you to shard Ironic nodes between compute \u0027"},{"line_number":92,"context_line":"             \u0027services across conductors and conductor groups. \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"b20ae2f1_bfffbcc8","line":89,"range":{"start_line":89,"start_character":14,"end_line":89,"end_character":33},"in_reply_to":"d9bfa582_17dee1fc","updated":"2024-02-27 15:38:36.000000000","message":"Implementing this broke/invalidated the test I added for the empty-string shard case, so I removed it.","commit_id":"5adaea90ed8259b0f5336212759cd543cd1ed65e"}],"nova/tests/unit/virt/ironic/test_driver.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"b8dcbd8aa29f300325137fad40cb9a8d40b3dddd","unresolved":true,"context_lines":[{"line_number":3153,"context_line":"        expected_cache \u003d {n.id: n for n in nodes}"},{"line_number":3154,"context_line":"        self.assertEqual(expected_cache, self.driver.node_cache)"},{"line_number":3155,"context_line":""},{"line_number":3156,"context_line":"    def test__refresh_cache_shard_and_conductor_group(self):"},{"line_number":3157,"context_line":"        # normal operation, one compute service"},{"line_number":3158,"context_line":"        instances \u003d []"},{"line_number":3159,"context_line":"        nodes \u003d ["}],"source_content_type":"text/x-python","patch_set":2,"id":"bde4a380_a1493ec5","line":3156,"updated":"2024-01-08 15:04:14.000000000","message":"can we add an explcit check for when shard\u003d\"\" with the conductor group set.\n\nwe shoudl expect 1.46 to be used not 1.82","commit_id":"5498fef8f65e53c466530382c7ed0d14588cba31"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0eb0bdb0f456e95df2bc665d8ec69288f7db7030","unresolved":false,"context_lines":[{"line_number":3153,"context_line":"        expected_cache \u003d {n.id: n for n in nodes}"},{"line_number":3154,"context_line":"        self.assertEqual(expected_cache, self.driver.node_cache)"},{"line_number":3155,"context_line":""},{"line_number":3156,"context_line":"    def test__refresh_cache_shard_and_conductor_group(self):"},{"line_number":3157,"context_line":"        # normal operation, one compute service"},{"line_number":3158,"context_line":"        instances \u003d []"},{"line_number":3159,"context_line":"        nodes \u003d ["}],"source_content_type":"text/x-python","patch_set":2,"id":"0b3f8dfb_8d0d1b23","line":3156,"in_reply_to":"bde4a380_a1493ec5","updated":"2024-02-25 12:00:35.000000000","message":"Done","commit_id":"5498fef8f65e53c466530382c7ed0d14588cba31"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"7109173fecf102b502215d3bc4d583037367b9dc","unresolved":false,"context_lines":[{"line_number":3153,"context_line":"        expected_cache \u003d {n.id: n for n in nodes}"},{"line_number":3154,"context_line":"        self.assertEqual(expected_cache, self.driver.node_cache)"},{"line_number":3155,"context_line":""},{"line_number":3156,"context_line":"    def test__refresh_cache_shard_and_conductor_group(self):"},{"line_number":3157,"context_line":"        # normal operation, one compute service"},{"line_number":3158,"context_line":"        instances \u003d []"},{"line_number":3159,"context_line":"        nodes \u003d ["}],"source_content_type":"text/x-python","patch_set":2,"id":"784e081a_1319d92f","line":3156,"in_reply_to":"bde4a380_a1493ec5","updated":"2024-02-25 21:13:39.000000000","message":"Done","commit_id":"5498fef8f65e53c466530382c7ed0d14588cba31"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0eb0bdb0f456e95df2bc665d8ec69288f7db7030","unresolved":true,"context_lines":[{"line_number":3194,"context_line":"        self.flags(conductor_group\u003dpartition_key, group\u003d\u0027ironic\u0027)"},{"line_number":3195,"context_line":""},{"line_number":3196,"context_line":"        self._test__refresh_cache(instances, nodes, hosts,"},{"line_number":3197,"context_line":"                                  shard\u003dshard, partition_key\u003dpartition_key, can_send_182\u003dFalse, can_send_146\u003dTrue)"},{"line_number":3198,"context_line":""},{"line_number":3199,"context_line":"        expected_cache \u003d {n.id: n for n in nodes}"},{"line_number":3200,"context_line":"        self.assertEqual(expected_cache, self.driver.node_cache)"}],"source_content_type":"text/x-python","patch_set":3,"id":"ce4fc1c2_b74f5c2d","line":3197,"in_reply_to":"054b2051_0b196cba","updated":"2024-02-25 12:00:35.000000000","message":"\u003e pep8: E501 line too long (114 \u003e 79 characters)\n\nPlease fix.","commit_id":"5adaea90ed8259b0f5336212759cd543cd1ed65e"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"3a5f1d9fbefd52cc24530384e17b727cfb2b3d1d","unresolved":false,"context_lines":[{"line_number":3194,"context_line":"        self.flags(conductor_group\u003dpartition_key, group\u003d\u0027ironic\u0027)"},{"line_number":3195,"context_line":""},{"line_number":3196,"context_line":"        self._test__refresh_cache(instances, nodes, hosts,"},{"line_number":3197,"context_line":"                                  shard\u003dshard, partition_key\u003dpartition_key, can_send_182\u003dFalse, can_send_146\u003dTrue)"},{"line_number":3198,"context_line":""},{"line_number":3199,"context_line":"        expected_cache \u003d {n.id: n for n in nodes}"},{"line_number":3200,"context_line":"        self.assertEqual(expected_cache, self.driver.node_cache)"}],"source_content_type":"text/x-python","patch_set":3,"id":"f5dd4345_8d93f86e","line":3197,"in_reply_to":"ce4fc1c2_b74f5c2d","updated":"2024-02-27 15:38:36.000000000","message":"Done","commit_id":"5adaea90ed8259b0f5336212759cd543cd1ed65e"}],"nova/virt/ironic/driver.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"b8dcbd8aa29f300325137fad40cb9a8d40b3dddd","unresolved":true,"context_lines":[{"line_number":698,"context_line":"            return False"},{"line_number":699,"context_line":""},{"line_number":700,"context_line":"    def _refresh_hash_ring(self, ctxt):"},{"line_number":701,"context_line":"        peer_list \u003d None"},{"line_number":702,"context_line":"        if CONF.ironic.shard is not None:"},{"line_number":703,"context_line":"            # When requesting a shard, we assume each compute service is"},{"line_number":704,"context_line":"            # targeting a separate shard, so hard code peer_list to"},{"line_number":705,"context_line":"            # just this service"},{"line_number":706,"context_line":"            peer_list \u003d set([CONF.host])"},{"line_number":707,"context_line":""},{"line_number":708,"context_line":"        # NOTE(jroll) if this is set, we need to limit the set of other"},{"line_number":709,"context_line":"        # compute services in the hash ring to hosts that are currently up"}],"source_content_type":"text/x-python","patch_set":2,"id":"1d4c4f17_dc745eb4","line":706,"range":{"start_line":701,"start_character":3,"end_line":706,"end_character":40},"updated":"2024-01-08 15:04:14.000000000","message":"i think this can be \n``` \n     # When requesting a shard, we assume each compute service is\n     # targeting a separate shard, so hard code peer_list to\n     # just this service\n     peer_list \u003d None if CONF.ironic.shard else {CONF.host}:\n```\nbut this is fine too.\n\nbool(CONF.ironic.shard) is false if  CONF.ironic.shard  is None or an empty string\nbool(CONF.ironic.shard) is true for any non empty string.\n\n\nso  \"if CONF.ironic.shard is not None:\" and  \"if CONF.ironic.shard:\" should be almost equivlent.\n\nif you explcitly set\n```\n[ironic]\nshard\u003d\"\"\n```\nbool(CONF.ironic.shard) would be false but CONF.ironic.shard is not non \u003d\u003d True\n\nassumign we want to fall back to the peerlist in that case we would need \n\n``` \n     # When requesting a shard, we assume each compute service is\n     # targeting a separate shard, so hard code peer_list to\n     # just this service\n     peer_list \u003d None if CONF.ironic.shard is not None else {CONF.host}:\n```\n\ninstead. that may or may not fit with the 79 column limit so treat this as a nit.","commit_id":"5498fef8f65e53c466530382c7ed0d14588cba31"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0eb0bdb0f456e95df2bc665d8ec69288f7db7030","unresolved":false,"context_lines":[{"line_number":698,"context_line":"            return False"},{"line_number":699,"context_line":""},{"line_number":700,"context_line":"    def _refresh_hash_ring(self, ctxt):"},{"line_number":701,"context_line":"        peer_list \u003d None"},{"line_number":702,"context_line":"        if CONF.ironic.shard is not None:"},{"line_number":703,"context_line":"            # When requesting a shard, we assume each compute service is"},{"line_number":704,"context_line":"            # targeting a separate shard, so hard code peer_list to"},{"line_number":705,"context_line":"            # just this service"},{"line_number":706,"context_line":"            peer_list \u003d set([CONF.host])"},{"line_number":707,"context_line":""},{"line_number":708,"context_line":"        # NOTE(jroll) if this is set, we need to limit the set of other"},{"line_number":709,"context_line":"        # compute services in the hash ring to hosts that are currently up"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba334229_e072162b","line":706,"range":{"start_line":701,"start_character":3,"end_line":706,"end_character":40},"in_reply_to":"1d4c4f17_dc745eb4","updated":"2024-02-25 12:00:35.000000000","message":"Acknowledged","commit_id":"5498fef8f65e53c466530382c7ed0d14588cba31"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"7109173fecf102b502215d3bc4d583037367b9dc","unresolved":false,"context_lines":[{"line_number":698,"context_line":"            return False"},{"line_number":699,"context_line":""},{"line_number":700,"context_line":"    def _refresh_hash_ring(self, ctxt):"},{"line_number":701,"context_line":"        peer_list \u003d None"},{"line_number":702,"context_line":"        if CONF.ironic.shard is not None:"},{"line_number":703,"context_line":"            # When requesting a shard, we assume each compute service is"},{"line_number":704,"context_line":"            # targeting a separate shard, so hard code peer_list to"},{"line_number":705,"context_line":"            # just this service"},{"line_number":706,"context_line":"            peer_list \u003d set([CONF.host])"},{"line_number":707,"context_line":""},{"line_number":708,"context_line":"        # NOTE(jroll) if this is set, we need to limit the set of other"},{"line_number":709,"context_line":"        # compute services in the hash ring to hosts that are currently up"}],"source_content_type":"text/x-python","patch_set":2,"id":"bbac5a08_4dca1f96","line":706,"range":{"start_line":701,"start_character":3,"end_line":706,"end_character":40},"in_reply_to":"1d4c4f17_dc745eb4","updated":"2024-02-25 21:13:39.000000000","message":"Done","commit_id":"5498fef8f65e53c466530382c7ed0d14588cba31"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"b8dcbd8aa29f300325137fad40cb9a8d40b3dddd","unresolved":false,"context_lines":[{"line_number":709,"context_line":"        # compute services in the hash ring to hosts that are currently up"},{"line_number":710,"context_line":"        # and specified in the peer_list config option, as there\u0027s no way"},{"line_number":711,"context_line":"        # to check which conductor_group other compute services are using."},{"line_number":712,"context_line":"        if peer_list is None and CONF.ironic.conductor_group is not None:"},{"line_number":713,"context_line":"            try:"},{"line_number":714,"context_line":"                # NOTE(jroll) first we need to make sure the Ironic API can"},{"line_number":715,"context_line":"                # filter by conductor_group. If it cannot, limiting to"}],"source_content_type":"text/x-python","patch_set":2,"id":"daa11e82_688c637d","line":712,"range":{"start_line":712,"start_character":11,"end_line":712,"end_character":28},"updated":"2024-01-08 15:04:14.000000000","message":"ok this will alwasy either be set to a value or None at this point\n\nthis could be \"if not peer_list and ...\" but this is more explict so ok.","commit_id":"5498fef8f65e53c466530382c7ed0d14588cba31"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"7109173fecf102b502215d3bc4d583037367b9dc","unresolved":false,"context_lines":[{"line_number":709,"context_line":"        # compute services in the hash ring to hosts that are currently up"},{"line_number":710,"context_line":"        # and specified in the peer_list config option, as there\u0027s no way"},{"line_number":711,"context_line":"        # to check which conductor_group other compute services are using."},{"line_number":712,"context_line":"        if peer_list is None and CONF.ironic.conductor_group is not None:"},{"line_number":713,"context_line":"            try:"},{"line_number":714,"context_line":"                # NOTE(jroll) first we need to make sure the Ironic API can"},{"line_number":715,"context_line":"                # filter by conductor_group. If it cannot, limiting to"}],"source_content_type":"text/x-python","patch_set":2,"id":"b02798d8_615f5dd8","line":712,"range":{"start_line":712,"start_character":11,"end_line":712,"end_character":28},"in_reply_to":"daa11e82_688c637d","updated":"2024-02-25 21:13:39.000000000","message":"I really dislike having a \"if X and Y is not None\" constructs as they can read a little weird; I\u0027m going to leave it as is.","commit_id":"5498fef8f65e53c466530382c7ed0d14588cba31"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"b8dcbd8aa29f300325137fad40cb9a8d40b3dddd","unresolved":true,"context_lines":[{"line_number":781,"context_line":"            if conductor_group is not None:"},{"line_number":782,"context_line":"                self._can_send_version(\u00271.46\u0027)"},{"line_number":783,"context_line":"                kwargs[\u0027conductor_group\u0027] \u003d conductor_group"},{"line_number":784,"context_line":"            if shard is not None:"},{"line_number":785,"context_line":"                self._can_send_version(\u00271.82\u0027)"},{"line_number":786,"context_line":"                kwargs[\u0027shard\u0027] \u003d shard"},{"line_number":787,"context_line":"            nodes \u003d _get_node_list(**kwargs)"}],"source_content_type":"text/x-python","patch_set":2,"id":"8f0ccfbf_c96733f1","line":784,"updated":"2024-01-08 15:04:14.000000000","message":"in this case \"if shard:\" might be more correct so that we dont send this and do the addtional api check for the empty string.\n\n```\n[ironic]\nshard\u003d\"\"\n```\n\nalternitively we could tighten the regex here\nhttps://review.opendev.org/c/openstack/nova/+/903915/2/nova/conf/ironic.py#89\n\nby changing \"regex\u003dr\u0027^[a-zA-Z0-9_.-]*$\u0027,\" to regex\u003dr\u0027^[a-zA-Z0-9_.-]+$\u0027,\n\n* is 0 or more \n+ is 1 or more\n\n```\n[ironic]\nshard\u003d\"\"\n```\nand \n```\n[ironic]\nshard\u003d\n```\n\nare parsed diffently by the way\nthe former results in the config var beign initalised to the empty string and the later results in it being None i belive.\n\nwe would need to check that however because we do not want to block\n\n```\n[ironic]\nshard\u003d\n```\nthat should be the same as\n\n```\n[ironic]\n#shard\u003d\n```","commit_id":"5498fef8f65e53c466530382c7ed0d14588cba31"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0eb0bdb0f456e95df2bc665d8ec69288f7db7030","unresolved":false,"context_lines":[{"line_number":781,"context_line":"            if conductor_group is not None:"},{"line_number":782,"context_line":"                self._can_send_version(\u00271.46\u0027)"},{"line_number":783,"context_line":"                kwargs[\u0027conductor_group\u0027] \u003d conductor_group"},{"line_number":784,"context_line":"            if shard is not None:"},{"line_number":785,"context_line":"                self._can_send_version(\u00271.82\u0027)"},{"line_number":786,"context_line":"                kwargs[\u0027shard\u0027] \u003d shard"},{"line_number":787,"context_line":"            nodes \u003d _get_node_list(**kwargs)"}],"source_content_type":"text/x-python","patch_set":2,"id":"0a48893f_c2dc2f06","line":784,"in_reply_to":"8f0ccfbf_c96733f1","updated":"2024-02-25 12:00:35.000000000","message":"Done","commit_id":"5498fef8f65e53c466530382c7ed0d14588cba31"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"7109173fecf102b502215d3bc4d583037367b9dc","unresolved":false,"context_lines":[{"line_number":781,"context_line":"            if conductor_group is not None:"},{"line_number":782,"context_line":"                self._can_send_version(\u00271.46\u0027)"},{"line_number":783,"context_line":"                kwargs[\u0027conductor_group\u0027] \u003d conductor_group"},{"line_number":784,"context_line":"            if shard is not None:"},{"line_number":785,"context_line":"                self._can_send_version(\u00271.82\u0027)"},{"line_number":786,"context_line":"                kwargs[\u0027shard\u0027] \u003d shard"},{"line_number":787,"context_line":"            nodes \u003d _get_node_list(**kwargs)"}],"source_content_type":"text/x-python","patch_set":2,"id":"f1ae7b93_b296ee23","line":784,"in_reply_to":"8f0ccfbf_c96733f1","updated":"2024-02-25 21:13:39.000000000","message":"Done","commit_id":"5498fef8f65e53c466530382c7ed0d14588cba31"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"7f225c6dfad7b699c783a12a35f7aee27e0ea3cf","unresolved":true,"context_lines":[{"line_number":787,"context_line":"            LOG.error(\u0027Required Ironic API version is not \u0027"},{"line_number":788,"context_line":"                      \u0027available to filter nodes by conductor group \u0027"},{"line_number":789,"context_line":"                      \u0027and shard.\u0027)"},{"line_number":790,"context_line":"            nodes \u003d _get_node_list(**kwargs)"},{"line_number":791,"context_line":""},{"line_number":792,"context_line":"        # NOTE(saga): As _get_node_list() will take a long"},{"line_number":793,"context_line":"        # time to return in large clusters we need to call it before"}],"source_content_type":"text/x-python","patch_set":4,"id":"4a7f7388_5977fbd8","line":790,"updated":"2024-02-28 17:49:19.000000000","message":"On reflection, maybe we should make this a hard failure, by replacing this line with \"raise\"?\n\nMostly because 1.82 was included in the previous SLURP release anyways. If there is no shard or conductor group set, no change, we just keep doing the same thing we usually do.","commit_id":"f1a4857d612cff0ecca1be3e914e57e9db6ac275"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"ddf4cc494d9111313604cb9fd5448acbad917cf1","unresolved":true,"context_lines":[{"line_number":787,"context_line":"            LOG.error(\u0027Required Ironic API version is not \u0027"},{"line_number":788,"context_line":"                      \u0027available to filter nodes by conductor group \u0027"},{"line_number":789,"context_line":"                      \u0027and shard.\u0027)"},{"line_number":790,"context_line":"            nodes \u003d _get_node_list(**kwargs)"},{"line_number":791,"context_line":""},{"line_number":792,"context_line":"        # NOTE(saga): As _get_node_list() will take a long"},{"line_number":793,"context_line":"        # time to return in large clusters we need to call it before"}],"source_content_type":"text/x-python","patch_set":4,"id":"a4d356d8_483752f9","line":790,"in_reply_to":"2a83d85f_086edfc8","updated":"2024-02-28 20:35:41.000000000","message":"I *am* opposed to doing it in init_host: we\u0027d be adding a hard dependency that Ironic API is started before n-cpu which does not exist today. \n\nOverall I think this is a decent idea, but likely has downsides that make me not thrilled with doing it at this point. I\u0027d rather release as it is, and convert this to a hard error for \"D\" cycle (or next slurp, E, depending on policy).","commit_id":"f1a4857d612cff0ecca1be3e914e57e9db6ac275"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"450135537bfbb3811235ab036eef3e433a2e8562","unresolved":true,"context_lines":[{"line_number":787,"context_line":"            LOG.error(\u0027Required Ironic API version is not \u0027"},{"line_number":788,"context_line":"                      \u0027available to filter nodes by conductor group \u0027"},{"line_number":789,"context_line":"                      \u0027and shard.\u0027)"},{"line_number":790,"context_line":"            nodes \u003d _get_node_list(**kwargs)"},{"line_number":791,"context_line":""},{"line_number":792,"context_line":"        # NOTE(saga): As _get_node_list() will take a long"},{"line_number":793,"context_line":"        # time to return in large clusters we need to call it before"}],"source_content_type":"text/x-python","patch_set":4,"id":"87502ebd_5a5b4c28","line":790,"in_reply_to":"4a7f7388_5977fbd8","updated":"2024-02-28 18:47:04.000000000","message":"I will push a follow-up patch to do this if consensus says to.\n\nThe only real case where this could happen and matter would be if someone was running an older openstacksdk version than in our U-C. \n- If you\u0027re really on an older Ironic, you can\u0027t set node shards on Ironic side and this doesn\u0027t matter.\n- If you\u0027ve manually installed an older SDK (I suspect this may be more breaky in other parts of nova codebase), it will just pass through here assuming it doesn\u0027t break elsewhere.\n\nI could go either way, I think raise vs log is mainly a style thing here -- it\u0027s extremely unlikely this case would be hit in a situation where being not sharded would matter.","commit_id":"f1a4857d612cff0ecca1be3e914e57e9db6ac275"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"33f84b54524951493273e9807abddc4c60547a77","unresolved":true,"context_lines":[{"line_number":787,"context_line":"            LOG.error(\u0027Required Ironic API version is not \u0027"},{"line_number":788,"context_line":"                      \u0027available to filter nodes by conductor group \u0027"},{"line_number":789,"context_line":"                      \u0027and shard.\u0027)"},{"line_number":790,"context_line":"            nodes \u003d _get_node_list(**kwargs)"},{"line_number":791,"context_line":""},{"line_number":792,"context_line":"        # NOTE(saga): As _get_node_list() will take a long"},{"line_number":793,"context_line":"        # time to return in large clusters we need to call it before"}],"source_content_type":"text/x-python","patch_set":4,"id":"2a83d85f_086edfc8","line":790,"in_reply_to":"87502ebd_5a5b4c28","updated":"2024-02-28 20:29:17.000000000","message":"im not oppesed to makign this a hard error.\ni tought we were mainly doing this to help with upgrades so that nova could be upgraded before ironic without requireing restart of nova.\n\ni think that is good to supprot but if you want to be more concervitive then i can see making it a hard error.\n\nthis seam kind of late to me however.\ni woudl expect to put a block in init_host if we were oging to do this. i.e. by check ing if we have the shard key in the config and if ironic is new enough.","commit_id":"f1a4857d612cff0ecca1be3e914e57e9db6ac275"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f45ffdbc40c4d393ed0981053805bd3e0b2bc7a9","unresolved":true,"context_lines":[{"line_number":787,"context_line":"            LOG.error(\u0027Required Ironic API version is not \u0027"},{"line_number":788,"context_line":"                      \u0027available to filter nodes by conductor group \u0027"},{"line_number":789,"context_line":"                      \u0027and shard.\u0027)"},{"line_number":790,"context_line":"            nodes \u003d _get_node_list(**kwargs)"},{"line_number":791,"context_line":""},{"line_number":792,"context_line":"        # NOTE(saga): As _get_node_list() will take a long"},{"line_number":793,"context_line":"        # time to return in large clusters we need to call it before"}],"source_content_type":"text/x-python","patch_set":4,"id":"7105b887_a6f87cdb","line":790,"in_reply_to":"a4d356d8_483752f9","updated":"2024-02-29 11:49:09.000000000","message":"well yes and no\n\nit would only be a adepnecy on ironci api if you have enabeld the ironic driver and you have set the shard key.\n\nI\u0027m ok with the fallback as is currently written but i defer to your an johns judgement.","commit_id":"f1a4857d612cff0ecca1be3e914e57e9db6ac275"}]}