)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"78552a0ea244d9a5feb24690b771446974ad9e56","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"f7be900e_77b886f8","updated":"2023-01-09 18:39:04.000000000","message":"i think this is a good start im holding +2 based on the open discussion if gibi and or others are fine with what dan has ill upgrade to +2 on this patch","commit_id":"c77cacaef420bfa8052d9f8015d87ca14e4c2be2"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"31cc53a5126eb18cb6b74a387f626179e893c964","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"fb5d2818_fb0ccf3b","updated":"2023-01-10 22:29:12.000000000","message":"recheck weird unrelated functional failure","commit_id":"8e1e88a11b516a9d76fe5eb472dcb761007ce831"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"93dd86a2032841483d3ed7fc7db4d78de2fea38e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"062e5ec8_103e3ed9","updated":"2023-01-13 00:21:44.000000000","message":"Looks OK to me. +1, there are other reviewers who may like to ack this PS.","commit_id":"2dec7d46790f489dcf1333c073065a6448630d31"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a2734b8b1e04153e8ee3be02c556290dba5ac86e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"764da480_c4386070","updated":"2023-01-18 06:28:25.000000000","message":"there are some pending comments so +1 as over all i think this is ok but ill wait for those to be adressed and review the rest fo the seriese","commit_id":"2dec7d46790f489dcf1333c073065a6448630d31"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ac57cdde20e4fb52fef51870bf21a63622f9fb98","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"ca6e96f7_5e503a07","updated":"2023-01-26 10:23:03.000000000","message":"I only have test related suggestions and those can be fixed in a FUP.","commit_id":"3b33b0938ea4f5b00537298bc7fcb531fb9ec811"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"070a54048e69357c52883cdf2b97691dd4f1b193","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"18fa05c3_f4a3ee59","updated":"2023-01-26 20:53:11.000000000","message":"i dont see any nits that cant be adressed in a fup sook im happy to move forward with this as is","commit_id":"3b33b0938ea4f5b00537298bc7fcb531fb9ec811"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4ee9eca9173be3f7d042c5f764647268561eaa54","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"aca0dbfd_d1acbedb","updated":"2023-01-24 01:13:31.000000000","message":"im still not sure if there are chagnes pendign for this patch or not.\n\ncan you make the comments as resovled/wont do ectra for the opens ones.\n","commit_id":"3b33b0938ea4f5b00537298bc7fcb531fb9ec811"}],"nova/test.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ac57cdde20e4fb52fef51870bf21a63622f9fb98","unresolved":true,"context_lines":[{"line_number":302,"context_line":""},{"line_number":303,"context_line":"        # Reset our local node uuid cache (and avoid writing to the"},{"line_number":304,"context_line":"        # local filesystem when we generate a new one)."},{"line_number":305,"context_line":"        node.LOCAL_NODE_UUID \u003d None"},{"line_number":306,"context_line":"        self.useFixture(nova_fixtures.ComputeNodeIdFixture())"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"    def _setup_cells(self):"},{"line_number":309,"context_line":"        \"\"\"Setup a normal cellsv2 environment."}],"source_content_type":"text/x-python","patch_set":7,"id":"04acc1df_979a79b0","line":306,"range":{"start_line":305,"start_character":1,"end_line":306,"end_character":61},"updated":"2023-01-26 10:23:03.000000000","message":"I suggest to move the global reset to the fixture to avoid missing one or the other.","commit_id":"3b33b0938ea4f5b00537298bc7fcb531fb9ec811"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"896cc717bd9a3562e29a903644988ef92c9e6023","unresolved":true,"context_lines":[{"line_number":302,"context_line":""},{"line_number":303,"context_line":"        # Reset our local node uuid cache (and avoid writing to the"},{"line_number":304,"context_line":"        # local filesystem when we generate a new one)."},{"line_number":305,"context_line":"        node.LOCAL_NODE_UUID \u003d None"},{"line_number":306,"context_line":"        self.useFixture(nova_fixtures.ComputeNodeIdFixture())"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"    def _setup_cells(self):"},{"line_number":309,"context_line":"        \"\"\"Setup a normal cellsv2 environment."}],"source_content_type":"text/x-python","patch_set":7,"id":"99894d04_6beec002","line":306,"range":{"start_line":305,"start_character":1,"end_line":306,"end_character":61},"in_reply_to":"04acc1df_979a79b0","updated":"2023-01-26 15:42:35.000000000","message":"Yeah you might be right. I\u0027ll poke at that as a FUP on top. I wanted to make sure that we\u0027re never leaking a UUID from the previous test, and we do several such resets here. Getting all the tests that simulate stuff running on lots of different nodes was a real struggle, but perhaps I was a little overzealous.","commit_id":"3b33b0938ea4f5b00537298bc7fcb531fb9ec811"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"293b1258506939f27d304b0dfbc5216ca44aa44c","unresolved":true,"context_lines":[{"line_number":302,"context_line":""},{"line_number":303,"context_line":"        # Reset our local node uuid cache (and avoid writing to the"},{"line_number":304,"context_line":"        # local filesystem when we generate a new one)."},{"line_number":305,"context_line":"        node.LOCAL_NODE_UUID \u003d None"},{"line_number":306,"context_line":"        self.useFixture(nova_fixtures.ComputeNodeIdFixture())"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"    def _setup_cells(self):"},{"line_number":309,"context_line":"        \"\"\"Setup a normal cellsv2 environment."}],"source_content_type":"text/x-python","patch_set":7,"id":"fad48abc_c7bcc025","line":306,"range":{"start_line":305,"start_character":1,"end_line":306,"end_character":61},"in_reply_to":"99894d04_6beec002","updated":"2023-01-26 19:20:12.000000000","message":"Oh heh, I wasn\u0027t even thinking in my pre-caffeinated state. Yes, of course.","commit_id":"3b33b0938ea4f5b00537298bc7fcb531fb9ec811"}],"nova/tests/fixtures/nova.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"78552a0ea244d9a5feb24690b771446974ad9e56","unresolved":true,"context_lines":[{"line_number":1806,"context_line":""},{"line_number":1807,"context_line":"        self.useFixture(fixtures.MockPatch("},{"line_number":1808,"context_line":"            \u0027nova.virt.node.read_local_node_uuid\u0027,"},{"line_number":1809,"context_line":"            lambda: None))"},{"line_number":1810,"context_line":"        self.useFixture(fixtures.MockPatch("},{"line_number":1811,"context_line":"            \u0027nova.virt.node.write_local_node_uuid\u0027,"},{"line_number":1812,"context_line":"            lambda uuid: None))"}],"source_content_type":"text/x-python","patch_set":2,"id":"4f8853da_7792e706","line":1809,"range":{"start_line":1809,"start_character":11,"end_line":1809,"end_character":24},"updated":"2023-01-09 18:39:04.000000000","message":"im not sure this is actully needed\n\nhttps://github.com/testing-cabal/fixtures/blob/master/fixtures/_fixtures/mockpatch.py#L47\n\ni think this is being passed as the argument to new.\n\nit wont hurt anything but you might be able to get away with\n\nself.useFixture(fixtures.MockPatch(\u0027nova.virt.node.read_local_node_uuid\u0027))","commit_id":"c77cacaef420bfa8052d9f8015d87ca14e4c2be2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c38d64b7e7268c9e4e094105f20c02d91f735149","unresolved":false,"context_lines":[{"line_number":1806,"context_line":""},{"line_number":1807,"context_line":"        self.useFixture(fixtures.MockPatch("},{"line_number":1808,"context_line":"            \u0027nova.virt.node.read_local_node_uuid\u0027,"},{"line_number":1809,"context_line":"            lambda: None))"},{"line_number":1810,"context_line":"        self.useFixture(fixtures.MockPatch("},{"line_number":1811,"context_line":"            \u0027nova.virt.node.write_local_node_uuid\u0027,"},{"line_number":1812,"context_line":"            lambda uuid: None))"}],"source_content_type":"text/x-python","patch_set":2,"id":"ed39bc1f_fe3432fa","line":1809,"range":{"start_line":1809,"start_character":11,"end_line":1809,"end_character":24},"in_reply_to":"180cc4ed_e1df4a81","updated":"2023-01-10 22:35:38.000000000","message":"ya using the lamda is fine dont worry about it.\n\ni was just not sure it was requried.\n\nif removing it is causeing issues leave it as is.","commit_id":"c77cacaef420bfa8052d9f8015d87ca14e4c2be2"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9472bf27dba6996964759f5cd3212278962bb11d","unresolved":true,"context_lines":[{"line_number":1806,"context_line":""},{"line_number":1807,"context_line":"        self.useFixture(fixtures.MockPatch("},{"line_number":1808,"context_line":"            \u0027nova.virt.node.read_local_node_uuid\u0027,"},{"line_number":1809,"context_line":"            lambda: None))"},{"line_number":1810,"context_line":"        self.useFixture(fixtures.MockPatch("},{"line_number":1811,"context_line":"            \u0027nova.virt.node.write_local_node_uuid\u0027,"},{"line_number":1812,"context_line":"            lambda uuid: None))"}],"source_content_type":"text/x-python","patch_set":2,"id":"e9f03212_8fdbf118","line":1809,"range":{"start_line":1809,"start_character":11,"end_line":1809,"end_character":24},"in_reply_to":"4f8853da_7792e706","updated":"2023-01-10 15:29:37.000000000","message":"I mean, I guess. Seems clearer on a quick read that I\u0027m mocking it with a NOP, but it doesn\u0027t matter that much.","commit_id":"c77cacaef420bfa8052d9f8015d87ca14e4c2be2"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"0dfdbbe63cc96507cf0f759434a17f046683159c","unresolved":true,"context_lines":[{"line_number":1806,"context_line":""},{"line_number":1807,"context_line":"        self.useFixture(fixtures.MockPatch("},{"line_number":1808,"context_line":"            \u0027nova.virt.node.read_local_node_uuid\u0027,"},{"line_number":1809,"context_line":"            lambda: None))"},{"line_number":1810,"context_line":"        self.useFixture(fixtures.MockPatch("},{"line_number":1811,"context_line":"            \u0027nova.virt.node.write_local_node_uuid\u0027,"},{"line_number":1812,"context_line":"            lambda uuid: None))"}],"source_content_type":"text/x-python","patch_set":2,"id":"180cc4ed_e1df4a81","line":1809,"range":{"start_line":1809,"start_character":11,"end_line":1809,"end_character":24},"in_reply_to":"e9f03212_8fdbf118","updated":"2023-01-10 19:27:40.000000000","message":"Actually, when I do this I get a failure to auto-spec another mock object. I\u0027m not entirely sure why, but what I really want is to replace this with a function that doesn\u0027t do the normal thing, so I think my lambda approach should be okay.","commit_id":"c77cacaef420bfa8052d9f8015d87ca14e4c2be2"}],"nova/tests/functional/integrated_helpers.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ac57cdde20e4fb52fef51870bf21a63622f9fb98","unresolved":true,"context_lines":[{"line_number":1230,"context_line":"        self.glance \u003d self.useFixture(nova_fixtures.GlanceFixture(self))"},{"line_number":1231,"context_line":"        self.policy \u003d self.useFixture(nova_fixtures.RealPolicyFixture())"},{"line_number":1232,"context_line":""},{"line_number":1233,"context_line":"        self.useFixture(nova_fixtures.ComputeNodeIdFixture())"},{"line_number":1234,"context_line":""},{"line_number":1235,"context_line":"        self.notifier \u003d self.useFixture("},{"line_number":1236,"context_line":"            nova_fixtures.NotificationFixture(self))"}],"source_content_type":"text/x-python","patch_set":7,"id":"7c87b6e8_f55eb4d8","line":1233,"updated":"2023-01-26 10:23:03.000000000","message":"hm we have this fixture added in test.TestCase so I\u0027m wondering if this is needed here.","commit_id":"3b33b0938ea4f5b00537298bc7fcb531fb9ec811"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ac57cdde20e4fb52fef51870bf21a63622f9fb98","unresolved":true,"context_lines":[{"line_number":1303,"context_line":"        self.placement \u003d self.useFixture(func_fixtures.PlacementFixture()).api"},{"line_number":1304,"context_line":"        self.useFixture(nova_fixtures.AllServicesCurrent())"},{"line_number":1305,"context_line":""},{"line_number":1306,"context_line":"        self.useFixture(nova_fixtures.ComputeNodeIdFixture())"},{"line_number":1307,"context_line":""},{"line_number":1308,"context_line":"        self.notifier \u003d self.useFixture("},{"line_number":1309,"context_line":"            nova_fixtures.NotificationFixture(self))"}],"source_content_type":"text/x-python","patch_set":7,"id":"8580f718_d58f3ef1","line":1306,"updated":"2023-01-26 10:23:03.000000000","message":"ditto","commit_id":"3b33b0938ea4f5b00537298bc7fcb531fb9ec811"}],"nova/tests/unit/virt/test_node.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"78552a0ea244d9a5feb24690b771446974ad9e56","unresolved":false,"context_lines":[{"line_number":32,"context_line":"# NOTE(danms): We do not inherit from test.TestCase because we need"},{"line_number":33,"context_line":"# our node methods not stubbed out in order to exercise them."},{"line_number":34,"context_line":"class TestNodeIdentity(testtools.TestCase):"},{"line_number":35,"context_line":"    def flags(self, **kw):"},{"line_number":36,"context_line":"        \"\"\"Override flag variables for a test.\"\"\""},{"line_number":37,"context_line":"        group \u003d kw.pop(\u0027group\u0027, None)"},{"line_number":38,"context_line":"        for k, v in kw.items():"}],"source_content_type":"text/x-python","patch_set":2,"id":"8af1180c_8c4995b6","line":35,"updated":"2023-01-09 18:39:04.000000000","message":"oh ok so here your are just replicating the flags interface we are sued to in \nin test.TestCase since we are using testtools.TestCase\n\nack","commit_id":"c77cacaef420bfa8052d9f8015d87ca14e4c2be2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"78552a0ea244d9a5feb24690b771446974ad9e56","unresolved":false,"context_lines":[{"line_number":68,"context_line":"            e \u003d self.assertRaises(exception.InvalidNodeConfiguration,"},{"line_number":69,"context_line":"                                  node.write_local_node_uuid, \u0027foo\u0027)"},{"line_number":70,"context_line":"            self.assertIn(\u0027Unable to write uuid to %s\u0027 % ("},{"line_number":71,"context_line":"                self.identity_file), str(e))"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    def test_generate_local_node_uuid_unexpected_write_fail(self):"},{"line_number":74,"context_line":"        with mock.patch(\u0027builtins.open\u0027) as mock_open:"}],"source_content_type":"text/x-python","patch_set":2,"id":"34e01bf5_629e709d","line":71,"updated":"2023-01-09 18:39:04.000000000","message":"ok this took a sec to parse this.\n\nyou are asserting the error contianer both the geneirc \"Unable to write uuid to %s\" message and the identify file path that was generated for this test execution.\n\nso the assert is validating two thing one that the error is as expect and to that the correct identity_file was being used and therefor the test setup is correct.","commit_id":"c77cacaef420bfa8052d9f8015d87ca14e4c2be2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"78552a0ea244d9a5feb24690b771446974ad9e56","unresolved":false,"context_lines":[{"line_number":76,"context_line":"            e \u003d self.assertRaises(exception.InvalidNodeConfiguration,"},{"line_number":77,"context_line":"                                  node.write_local_node_uuid, \u0027foo\u0027)"},{"line_number":78,"context_line":"            self.assertIn(\u0027Unable to write uuid to %s\u0027 % ("},{"line_number":79,"context_line":"                self.identity_file), str(e))"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def test_get_local_node_uuid_simple_exists(self):"},{"line_number":82,"context_line":"        node_uuid \u003d uuids.node"}],"source_content_type":"text/x-python","patch_set":2,"id":"18105bb8_cd5a4d68","line":79,"updated":"2023-01-09 18:39:04.000000000","message":"and then the same but on write rather then open.\n\ni doubt that would raise an IndexError but the exction is less imporant then the sideffect which is what you are testing. it will proably be some subclass of oserror","commit_id":"c77cacaef420bfa8052d9f8015d87ca14e4c2be2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"78552a0ea244d9a5feb24690b771446974ad9e56","unresolved":true,"context_lines":[{"line_number":92,"context_line":"            self.assertEqual(node_uuid, node.get_local_node_uuid())"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    def test_get_local_node_uuid_simple_generate(self):"},{"line_number":95,"context_line":"        self.assertIsNone(node.LOCAL_NODE_UUID)"},{"line_number":96,"context_line":"        node_uuid1 \u003d node.get_local_node_uuid()"},{"line_number":97,"context_line":"        self.assertEqual(node_uuid1, node.LOCAL_NODE_UUID)"},{"line_number":98,"context_line":"        node_uuid2 \u003d node.get_local_node_uuid()"},{"line_number":99,"context_line":"        self.assertEqual(node_uuid1, node.LOCAL_NODE_UUID)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"        # Make sure we got the same thing each time, and that it\u0027s a"},{"line_number":102,"context_line":"        # valid uuid. Since we provided no uuid, it must have been"},{"line_number":103,"context_line":"        # generated the first time and read/returned the second."},{"line_number":104,"context_line":"        self.assertEqual(node_uuid1, node_uuid2)"},{"line_number":105,"context_line":"        uuid.UUID(node_uuid1)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        # Try to read it directly to make sure the file was really"}],"source_content_type":"text/x-python","patch_set":2,"id":"71e8868d_800520bb","line":104,"range":{"start_line":95,"start_character":2,"end_line":104,"end_character":48},"updated":"2023-01-09 18:39:04.000000000","message":"nit: i would group this slitly differnt\n\n```\n        # Make sure we got the same thing each time.\n        self.assertIsNone(node.LOCAL_NODE_UUID)\n        node_uuid1 \u003d node.get_local_node_uuid()\n        self.assertEqual(node_uuid1, node.LOCAL_NODE_UUID)\n        node_uuid2 \u003d node.get_local_node_uuid()\n        self.assertEqual(node_uuid1, node.LOCAL_NODE_UUID)\n        self.assertEqual(node_uuid1, node_uuid2)\n \n        # and that it\u0027s a valid uuid.\n        uuid.UUID(node_uuid1)\n```     \n        \nthat said the logic is the same i just find the equaltiy check between\nnode_uuid1 and  node_uuid2 to be more closely corralated withteh first group of asserts then they uuid validity check.","commit_id":"c77cacaef420bfa8052d9f8015d87ca14e4c2be2"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9472bf27dba6996964759f5cd3212278962bb11d","unresolved":true,"context_lines":[{"line_number":92,"context_line":"            self.assertEqual(node_uuid, node.get_local_node_uuid())"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    def test_get_local_node_uuid_simple_generate(self):"},{"line_number":95,"context_line":"        self.assertIsNone(node.LOCAL_NODE_UUID)"},{"line_number":96,"context_line":"        node_uuid1 \u003d node.get_local_node_uuid()"},{"line_number":97,"context_line":"        self.assertEqual(node_uuid1, node.LOCAL_NODE_UUID)"},{"line_number":98,"context_line":"        node_uuid2 \u003d node.get_local_node_uuid()"},{"line_number":99,"context_line":"        self.assertEqual(node_uuid1, node.LOCAL_NODE_UUID)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"        # Make sure we got the same thing each time, and that it\u0027s a"},{"line_number":102,"context_line":"        # valid uuid. Since we provided no uuid, it must have been"},{"line_number":103,"context_line":"        # generated the first time and read/returned the second."},{"line_number":104,"context_line":"        self.assertEqual(node_uuid1, node_uuid2)"},{"line_number":105,"context_line":"        uuid.UUID(node_uuid1)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        # Try to read it directly to make sure the file was really"}],"source_content_type":"text/x-python","patch_set":2,"id":"906178ba_302a0a79","line":104,"range":{"start_line":95,"start_character":2,"end_line":104,"end_character":48},"in_reply_to":"71e8868d_800520bb","updated":"2023-01-10 15:29:37.000000000","message":"Oh, I\u0027m quite sure I meant to assert node_uuid2 on L99\n\nTo me, making sure that we got the LOCAL_NODE_UUID each time we\u0027re making the call makes the most sense, and then to make sure they were the same after we\u0027ve gone through what might have been the generation part is the most straightforward. But see what you think with the fixed assertion on L99.","commit_id":"c77cacaef420bfa8052d9f8015d87ca14e4c2be2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4ee9eca9173be3f7d042c5f764647268561eaa54","unresolved":false,"context_lines":[{"line_number":92,"context_line":"            self.assertEqual(node_uuid, node.get_local_node_uuid())"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    def test_get_local_node_uuid_simple_generate(self):"},{"line_number":95,"context_line":"        self.assertIsNone(node.LOCAL_NODE_UUID)"},{"line_number":96,"context_line":"        node_uuid1 \u003d node.get_local_node_uuid()"},{"line_number":97,"context_line":"        self.assertEqual(node_uuid1, node.LOCAL_NODE_UUID)"},{"line_number":98,"context_line":"        node_uuid2 \u003d node.get_local_node_uuid()"},{"line_number":99,"context_line":"        self.assertEqual(node_uuid1, node.LOCAL_NODE_UUID)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"        # Make sure we got the same thing each time, and that it\u0027s a"},{"line_number":102,"context_line":"        # valid uuid. Since we provided no uuid, it must have been"},{"line_number":103,"context_line":"        # generated the first time and read/returned the second."},{"line_number":104,"context_line":"        self.assertEqual(node_uuid1, node_uuid2)"},{"line_number":105,"context_line":"        uuid.UUID(node_uuid1)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        # Try to read it directly to make sure the file was really"}],"source_content_type":"text/x-python","patch_set":2,"id":"3897a211_c1261bc2","line":104,"range":{"start_line":95,"start_character":2,"end_line":104,"end_character":48},"in_reply_to":"906178ba_302a0a79","updated":"2023-01-24 01:13:31.000000000","message":"ya the new grouping and fixed asserrt works for me thanks","commit_id":"c77cacaef420bfa8052d9f8015d87ca14e4c2be2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"78552a0ea244d9a5feb24690b771446974ad9e56","unresolved":true,"context_lines":[{"line_number":121,"context_line":"        self.assertEqual(node_uuid, node.get_local_node_uuid())"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"    def test_get_local_node_uuid_two_mismatch(self):"},{"line_number":124,"context_line":"        node_uuids \u003d [uuids.node1, uuids.node2]"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"        # Write a different uuid to each file"},{"line_number":127,"context_line":"        for i, cf in enumerate((self.fake_config_files[0],"},{"line_number":128,"context_line":"                                self.fake_config_files[1])):"},{"line_number":129,"context_line":"            open(os.path.join("},{"line_number":130,"context_line":"                os.path.dirname(cf),"},{"line_number":131,"context_line":"                node.COMPUTE_ID_FILE), \u0027w\u0027).write(node_uuids[i])"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"        # Make sure we get an error that identifies the mismatching"},{"line_number":134,"context_line":"        # file with its uuid, as well as what we expected to find"}],"source_content_type":"text/x-python","patch_set":2,"id":"aafaf447_9ad62b96","line":131,"range":{"start_line":124,"start_character":8,"end_line":131,"end_character":64},"updated":"2023-01-09 18:39:04.000000000","message":"i want to say this is a good usecase for zip\n```\nnode_uuids \u003d [uuids.node1, uuids.node2]\nfiles \u003d fake_config_files[:2]\nfor uuid, file in zip(node_uuids,files):\n   with open(os.path.join(os.path.dirname(cf), node.COMPUTE_ID_FILE), \u0027w\u0027) as f:\n                f.write(node_uuids[i])\n                \n```\n\nwill open as used in your current vesion actully close the file like object?\ni think this will currenlty leak the file descroptor and python will clean it up when the test runner process exits. i think this captured in the verbose output of stestr but i am not sure.\n\nyou are also relying on the tempdir fixture for cleanup of the files which is fine but that is technically spereate from the file descriptor which i think is leaked.\n\nlater\n-----\ni see this came up else where too and thsi is safe but not the style im used too.","commit_id":"c77cacaef420bfa8052d9f8015d87ca14e4c2be2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4ee9eca9173be3f7d042c5f764647268561eaa54","unresolved":false,"context_lines":[{"line_number":121,"context_line":"        self.assertEqual(node_uuid, node.get_local_node_uuid())"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"    def test_get_local_node_uuid_two_mismatch(self):"},{"line_number":124,"context_line":"        node_uuids \u003d [uuids.node1, uuids.node2]"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"        # Write a different uuid to each file"},{"line_number":127,"context_line":"        for i, cf in enumerate((self.fake_config_files[0],"},{"line_number":128,"context_line":"                                self.fake_config_files[1])):"},{"line_number":129,"context_line":"            open(os.path.join("},{"line_number":130,"context_line":"                os.path.dirname(cf),"},{"line_number":131,"context_line":"                node.COMPUTE_ID_FILE), \u0027w\u0027).write(node_uuids[i])"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"        # Make sure we get an error that identifies the mismatching"},{"line_number":134,"context_line":"        # file with its uuid, as well as what we expected to find"}],"source_content_type":"text/x-python","patch_set":2,"id":"a37217d4_f3890b49","line":131,"range":{"start_line":124,"start_character":8,"end_line":131,"end_character":64},"in_reply_to":"aafaf447_9ad62b96","updated":"2023-01-24 01:13:31.000000000","message":"Ack","commit_id":"c77cacaef420bfa8052d9f8015d87ca14e4c2be2"}],"nova/virt/node.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"9dfeeefb9078a3b33f3d7dc21754efefd2fc99fb","unresolved":true,"context_lines":[{"line_number":62,"context_line":"            # a little more to be graceful about whitespace in/around"},{"line_number":63,"context_line":"            # the actual value we want to read. However, it must parse"},{"line_number":64,"context_line":"            # to a legit UUID once we strip the whitespace."},{"line_number":65,"context_line":"            content \u003d open(fn).read(40)"},{"line_number":66,"context_line":"            node_uuid \u003d str(uuid.UUID(content.strip()))"},{"line_number":67,"context_line":"        except FileNotFoundError:"},{"line_number":68,"context_line":"            continue"}],"source_content_type":"text/x-python","patch_set":1,"id":"9d5bded1_147d9965","line":65,"updated":"2022-12-05 15:37:21.000000000","message":"it is probably fine but we are leaking open file description here.","commit_id":"d6631ccd6b6690b0d6ad30ebf13e1b1fa8b03d2f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"02b284b285e5755996ab044ea051ed77b4cac380","unresolved":true,"context_lines":[{"line_number":62,"context_line":"            # a little more to be graceful about whitespace in/around"},{"line_number":63,"context_line":"            # the actual value we want to read. However, it must parse"},{"line_number":64,"context_line":"            # to a legit UUID once we strip the whitespace."},{"line_number":65,"context_line":"            content \u003d open(fn).read(40)"},{"line_number":66,"context_line":"            node_uuid \u003d str(uuid.UUID(content.strip()))"},{"line_number":67,"context_line":"        except FileNotFoundError:"},{"line_number":68,"context_line":"            continue"}],"source_content_type":"text/x-python","patch_set":1,"id":"6b3f5d39_d99a6ed6","line":65,"in_reply_to":"4a90440c_503d3bba","updated":"2023-01-10 09:09:03.000000000","message":"OK, I\u0027m blindly applied the \"always explicitly handle the close of a file descriptor\" rule. \n\nI think in this case we rely on the garbage collector to collect file descriptor and during that collection the fd is closed. \n\nAs far as I know cpython has a refcount based primary garbage collector and a secondary collector for cyclic references. Here we can be sure that the temporary fd will not have a cyclic ref and only goes from refcount 1 to 0 during the open().read() line. So cpython will close this after read() (or after the exception raised by the read() is handled up somewhere in the stack that can be arbitrary far from this line).\n\nAs far as I know pypy does not have a refcount based collector just a generational one. So under pypy the open().read() line will not close the fd immediately it will be closed when the gc run, and that gc probably run only at memory pressure. \n\n```\ndef read():\n    open(\"a.txt\").read(40)\n\nif __name__ \u003d\u003d \"__main__\":\n    while True:\n        read()\n```\n\nThis code will have at most 1 open fd to a.txt under cpython and will have around 5k - 15k open fd to a.txt under pypy on my machine.\n\nSo as I said above it is probably fine as is. But we rely on the deep internal of the python interpreter *implementation* that can change. So the general rule of always explicitly close an open file has a power too as it would make the two python interpreters execute the same code with the same external resource constraints.","commit_id":"d6631ccd6b6690b0d6ad30ebf13e1b1fa8b03d2f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5c67060cfe64323af3fe75e771f639976667dfe4","unresolved":true,"context_lines":[{"line_number":62,"context_line":"            # a little more to be graceful about whitespace in/around"},{"line_number":63,"context_line":"            # the actual value we want to read. However, it must parse"},{"line_number":64,"context_line":"            # to a legit UUID once we strip the whitespace."},{"line_number":65,"context_line":"            content \u003d open(fn).read(40)"},{"line_number":66,"context_line":"            node_uuid \u003d str(uuid.UUID(content.strip()))"},{"line_number":67,"context_line":"        except FileNotFoundError:"},{"line_number":68,"context_line":"            continue"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf1e63a8_5cc210b7","line":65,"in_reply_to":"61cbfcae_417669b6","updated":"2023-01-10 14:37:18.000000000","message":"Yep, easy to just change this, so I will.","commit_id":"d6631ccd6b6690b0d6ad30ebf13e1b1fa8b03d2f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d975e12cc619c8be75bf7d38332115b52cce4114","unresolved":true,"context_lines":[{"line_number":62,"context_line":"            # a little more to be graceful about whitespace in/around"},{"line_number":63,"context_line":"            # the actual value we want to read. However, it must parse"},{"line_number":64,"context_line":"            # to a legit UUID once we strip the whitespace."},{"line_number":65,"context_line":"            content \u003d open(fn).read(40)"},{"line_number":66,"context_line":"            node_uuid \u003d str(uuid.UUID(content.strip()))"},{"line_number":67,"context_line":"        except FileNotFoundError:"},{"line_number":68,"context_line":"            continue"}],"source_content_type":"text/x-python","patch_set":1,"id":"61cbfcae_417669b6","line":65,"in_reply_to":"6b3f5d39_d99a6ed6","updated":"2023-01-10 12:02:00.000000000","message":"i prefer to use a with statement too expcitly close too.\n\nthe delta in pypy vs cpython behavior was one of the thing i was  concerned about.\n\nthat said out side fo the client which are ment to supprot pypy we only suport cpython in core nova.  that said if nova-compute ran on pypy i dont think enabling that for those that want it is harmful so i would not like to make that harder to achive.","commit_id":"d6631ccd6b6690b0d6ad30ebf13e1b1fa8b03d2f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"65039843c2be2963d4ad0524ca18a36414f65c9c","unresolved":true,"context_lines":[{"line_number":62,"context_line":"            # a little more to be graceful about whitespace in/around"},{"line_number":63,"context_line":"            # the actual value we want to read. However, it must parse"},{"line_number":64,"context_line":"            # to a legit UUID once we strip the whitespace."},{"line_number":65,"context_line":"            content \u003d open(fn).read(40)"},{"line_number":66,"context_line":"            node_uuid \u003d str(uuid.UUID(content.strip()))"},{"line_number":67,"context_line":"        except FileNotFoundError:"},{"line_number":68,"context_line":"            continue"}],"source_content_type":"text/x-python","patch_set":1,"id":"a8edec88_53d8010f","line":65,"in_reply_to":"8dfe6a67_7f480e43","updated":"2023-01-09 18:55:04.000000000","message":"\u003e i belive in py27 this would have been a leak but its not anymore which is proably why all the example still use with statements\n\nNo, the same test above works in py27 as well (just re-confirmed). Perhaps it\u0027s just only seen in some circles, but I do this *all* the time.","commit_id":"d6631ccd6b6690b0d6ad30ebf13e1b1fa8b03d2f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ceb6dcdde82ea6d045aec82cb0585381df2572f8","unresolved":true,"context_lines":[{"line_number":62,"context_line":"            # a little more to be graceful about whitespace in/around"},{"line_number":63,"context_line":"            # the actual value we want to read. However, it must parse"},{"line_number":64,"context_line":"            # to a legit UUID once we strip the whitespace."},{"line_number":65,"context_line":"            content \u003d open(fn).read(40)"},{"line_number":66,"context_line":"            node_uuid \u003d str(uuid.UUID(content.strip()))"},{"line_number":67,"context_line":"        except FileNotFoundError:"},{"line_number":68,"context_line":"            continue"}],"source_content_type":"text/x-python","patch_set":1,"id":"a68dcfb6_216b3a44","line":65,"in_reply_to":"9d5bded1_147d9965","updated":"2022-12-06 19:19:48.000000000","message":"I think it\u0027s closed immediately, especially if you never set the file object to a variable:\n\n dan@theobromine ~/glance$ python3                                                                             \n Python 3.10.4 (main, Mar 25 2022, 00:00:00) [GCC 11.2.1 20220127 \n (Red Hat 11.2.1-9)] on linux\n Type \"help\", \"copyright\", \"credits\" or \"license\" for more \ninformation.\n \u003e\u003e\u003e open(\u0027/etc/passwd\u0027).read()[0]\n \u0027r\u0027\n \u003e\u003e\u003e\n [1]  + 67099 suspended  python3\n dan@theobromine ~/glance$ ls -l /proc/67099/fd                                                         \n total 0\n lrwx------ 1 dan dan 64 Dec  6 11:18 0 -\u003e /dev/pts/0\n lrwx------ 1 dan dan 64 Dec  6 11:18 1 -\u003e /dev/pts/0\n lrwx------ 1 dan dan 64 Dec  6 11:18 2 -\u003e /dev/pts/0\n \nI do this quite a bit, but maybe I\u0027m being dumb. Do you have a reference?\n\nAlso happy to change it of course :)","commit_id":"d6631ccd6b6690b0d6ad30ebf13e1b1fa8b03d2f"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"69c03f785e5a61e65af7ad13a6023a4ddad2f4cb","unresolved":true,"context_lines":[{"line_number":62,"context_line":"            # a little more to be graceful about whitespace in/around"},{"line_number":63,"context_line":"            # the actual value we want to read. However, it must parse"},{"line_number":64,"context_line":"            # to a legit UUID once we strip the whitespace."},{"line_number":65,"context_line":"            content \u003d open(fn).read(40)"},{"line_number":66,"context_line":"            node_uuid \u003d str(uuid.UUID(content.strip()))"},{"line_number":67,"context_line":"        except FileNotFoundError:"},{"line_number":68,"context_line":"            continue"}],"source_content_type":"text/x-python","patch_set":1,"id":"f52c6b2b_c819421c","line":65,"in_reply_to":"a68dcfb6_216b3a44","updated":"2022-12-07 09:08:32.000000000","message":"Yes I think we are good with this dansmith\u0027s way :-) it\u0027s perhaps a bit hard as the fd got released by garbage collector instead of the graceful function.\n\nThe problem is when you keep a reference of the fd.","commit_id":"d6631ccd6b6690b0d6ad30ebf13e1b1fa8b03d2f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"cbfd43c2489d2b03224893e0bea2dce7ae2d54a4","unresolved":true,"context_lines":[{"line_number":62,"context_line":"            # a little more to be graceful about whitespace in/around"},{"line_number":63,"context_line":"            # the actual value we want to read. However, it must parse"},{"line_number":64,"context_line":"            # to a legit UUID once we strip the whitespace."},{"line_number":65,"context_line":"            content \u003d open(fn).read(40)"},{"line_number":66,"context_line":"            node_uuid \u003d str(uuid.UUID(content.strip()))"},{"line_number":67,"context_line":"        except FileNotFoundError:"},{"line_number":68,"context_line":"            continue"}],"source_content_type":"text/x-python","patch_set":1,"id":"4a90440c_503d3bba","line":65,"in_reply_to":"a8edec88_53d8010f","updated":"2023-01-09 20:26:59.000000000","message":"ok i guess the behaivor in the 2.7 docs is not as clear ill upgrade to a +2 then","commit_id":"d6631ccd6b6690b0d6ad30ebf13e1b1fa8b03d2f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ded2df38bed092e93f646a589a44e4227b25da80","unresolved":false,"context_lines":[{"line_number":62,"context_line":"            # a little more to be graceful about whitespace in/around"},{"line_number":63,"context_line":"            # the actual value we want to read. However, it must parse"},{"line_number":64,"context_line":"            # to a legit UUID once we strip the whitespace."},{"line_number":65,"context_line":"            content \u003d open(fn).read(40)"},{"line_number":66,"context_line":"            node_uuid \u003d str(uuid.UUID(content.strip()))"},{"line_number":67,"context_line":"        except FileNotFoundError:"},{"line_number":68,"context_line":"            continue"}],"source_content_type":"text/x-python","patch_set":1,"id":"d9540d37_8d88f992","line":65,"in_reply_to":"bf1e63a8_5cc210b7","updated":"2023-01-20 16:06:31.000000000","message":"Done","commit_id":"d6631ccd6b6690b0d6ad30ebf13e1b1fa8b03d2f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"78552a0ea244d9a5feb24690b771446974ad9e56","unresolved":true,"context_lines":[{"line_number":62,"context_line":"            # a little more to be graceful about whitespace in/around"},{"line_number":63,"context_line":"            # the actual value we want to read. However, it must parse"},{"line_number":64,"context_line":"            # to a legit UUID once we strip the whitespace."},{"line_number":65,"context_line":"            content \u003d open(fn).read(40)"},{"line_number":66,"context_line":"            node_uuid \u003d str(uuid.UUID(content.strip()))"},{"line_number":67,"context_line":"        except FileNotFoundError:"},{"line_number":68,"context_line":"            continue"}],"source_content_type":"text/x-python","patch_set":1,"id":"8dfe6a67_7f480e43","line":65,"in_reply_to":"f52c6b2b_c819421c","updated":"2023-01-09 18:39:04.000000000","message":"ah i see this came up here too.\ni would prefer to wrap the open fucntion call in  a with statement.\n\nyou do appear to be correct that if we never asign the file like object to a local with open(...) as f:\n   f.do_something()\n   \n shoudl be more portable.\n \ni expected the fd to be leaked as well so its not obvious to me that open().do_something() is actully safe.\n\n\nlooking at https://docs.python.org/3/library/functions.html#open\n\ni think this might be a delta between the open() built in and the lowerlevel os.open() which returns the file descriptor for the newly opened file. \n \nlookign at the built in open the actual fd closing behavior is coming form \nthe closefd paramater\n\nopen(file, mode\u003d\u0027r\u0027, buffering\u003d- 1, encoding\u003dNone, errors\u003dNone, newline\u003dNone, closefd\u003dTrue, opener\u003dNone)\n\n\nIf closefd is False and a file descriptor rather than a filename was given, the underlying file descriptor will be kept open when the file is closed. If a filename is given closefd must be True (the default); otherwise, an error will be raised.\n\nso dan is correct that this is safe the behaivor changed here between python 2.7 and python 3\nhttps://docs.python.org/2.7/library/functions.html#open\n\ni belive in py27 this would have been a leak but its not anymore which is proably why all the example still use with statements","commit_id":"d6631ccd6b6690b0d6ad30ebf13e1b1fa8b03d2f"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"69c03f785e5a61e65af7ad13a6023a4ddad2f4cb","unresolved":true,"context_lines":[{"line_number":68,"context_line":"            continue"},{"line_number":69,"context_line":"        except ValueError:"},{"line_number":70,"context_line":"            raise exception.InvalidNodeConfiguration("},{"line_number":71,"context_line":"                reason\u003d\u0027Unable to parse UUID from %s\u0027 % fn)"},{"line_number":72,"context_line":"        uuids.append(node_uuid)"},{"line_number":73,"context_line":"        found.append(fn)"},{"line_number":74,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"a6a58927_91b2b618","line":71,"updated":"2022-12-07 09:08:32.000000000","message":"Perhaps you can pass in the reason content.strip() That may help debugging if something wrong happen.","commit_id":"d6631ccd6b6690b0d6ad30ebf13e1b1fa8b03d2f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"070a54048e69357c52883cdf2b97691dd4f1b193","unresolved":false,"context_lines":[{"line_number":68,"context_line":"            continue"},{"line_number":69,"context_line":"        except ValueError:"},{"line_number":70,"context_line":"            raise exception.InvalidNodeConfiguration("},{"line_number":71,"context_line":"                reason\u003d\u0027Unable to parse UUID from %s\u0027 % fn)"},{"line_number":72,"context_line":"        uuids.append(node_uuid)"},{"line_number":73,"context_line":"        found.append(fn)"},{"line_number":74,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5d776645_7ba81da9","line":71,"in_reply_to":"79701243_b1465dd4","updated":"2023-01-26 20:53:11.000000000","message":"Ack","commit_id":"d6631ccd6b6690b0d6ad30ebf13e1b1fa8b03d2f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9472bf27dba6996964759f5cd3212278962bb11d","unresolved":true,"context_lines":[{"line_number":68,"context_line":"            continue"},{"line_number":69,"context_line":"        except ValueError:"},{"line_number":70,"context_line":"            raise exception.InvalidNodeConfiguration("},{"line_number":71,"context_line":"                reason\u003d\u0027Unable to parse UUID from %s\u0027 % fn)"},{"line_number":72,"context_line":"        uuids.append(node_uuid)"},{"line_number":73,"context_line":"        found.append(fn)"},{"line_number":74,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"79701243_b1465dd4","line":71,"in_reply_to":"a6a58927_91b2b618","updated":"2023-01-10 15:29:37.000000000","message":"I dunno, maybe, but it makes the error message long if there\u0027s something unexpected in there. Since I have the filename, and UUIDs are pretty well-understood I think it\u0027s probably easy to determine why it failed to parse.","commit_id":"d6631ccd6b6690b0d6ad30ebf13e1b1fa8b03d2f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"9dfeeefb9078a3b33f3d7dc21754efefd2fc99fb","unresolved":true,"context_lines":[{"line_number":79,"context_line":"            if node_uuid !\u003d first:"},{"line_number":80,"context_line":"                raise exception.InvalidNodeConfiguration("},{"line_number":81,"context_line":"                    reason\u003d\u0027UUID %s in %s does not match %s\u0027 % ("},{"line_number":82,"context_line":"                        node_uuid, fn, uuids[i - 1]))"},{"line_number":83,"context_line":"        LOG.info(\u0027Determined node identity %s from %s\u0027, first, found[0])"},{"line_number":84,"context_line":"        return first"},{"line_number":85,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"ca1d4d66_45f7a47a","line":82,"range":{"start_line":82,"start_character":38,"end_line":82,"end_character":51},"updated":"2022-12-05 15:37:21.000000000","message":"I think this should be first as we comparing against first","commit_id":"d6631ccd6b6690b0d6ad30ebf13e1b1fa8b03d2f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ceb6dcdde82ea6d045aec82cb0585381df2572f8","unresolved":false,"context_lines":[{"line_number":79,"context_line":"            if node_uuid !\u003d first:"},{"line_number":80,"context_line":"                raise exception.InvalidNodeConfiguration("},{"line_number":81,"context_line":"                    reason\u003d\u0027UUID %s in %s does not match %s\u0027 % ("},{"line_number":82,"context_line":"                        node_uuid, fn, uuids[i - 1]))"},{"line_number":83,"context_line":"        LOG.info(\u0027Determined node identity %s from %s\u0027, first, found[0])"},{"line_number":84,"context_line":"        return first"},{"line_number":85,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"0d195407_1db238e5","line":82,"range":{"start_line":82,"start_character":38,"end_line":82,"end_character":51},"in_reply_to":"ca1d4d66_45f7a47a","updated":"2022-12-06 19:19:48.000000000","message":"Ack","commit_id":"d6631ccd6b6690b0d6ad30ebf13e1b1fa8b03d2f"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"bd31efd7ee3c6ae110b6e20f117a2c949acba68f","unresolved":true,"context_lines":[{"line_number":73,"context_line":"        uuids.append(node_uuid)"},{"line_number":74,"context_line":"        found.append(fn)"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    if uuids:"},{"line_number":77,"context_line":"        # Any identities we found must be consistent, or we fail"},{"line_number":78,"context_line":"        first \u003d uuids[0]"},{"line_number":79,"context_line":"        for i, (node_uuid, fn) in enumerate(zip(uuids, found)):"}],"source_content_type":"text/x-python","patch_set":3,"id":"5bc9dd62_dff8ea83","line":76,"updated":"2023-01-10 18:57:26.000000000","message":"I wonder if a more elegant way would be to make a set and assert its size is 1, but meh.","commit_id":"28bf33ddc59e551445ac999cc41b841a4c49b4d8"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"070a54048e69357c52883cdf2b97691dd4f1b193","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        uuids.append(node_uuid)"},{"line_number":74,"context_line":"        found.append(fn)"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    if uuids:"},{"line_number":77,"context_line":"        # Any identities we found must be consistent, or we fail"},{"line_number":78,"context_line":"        first \u003d uuids[0]"},{"line_number":79,"context_line":"        for i, (node_uuid, fn) in enumerate(zip(uuids, found)):"}],"source_content_type":"text/x-python","patch_set":3,"id":"2f9dc554_2e2526de","line":76,"in_reply_to":"46e9d054_a80d2347","updated":"2023-01-26 20:53:11.000000000","message":"Ack","commit_id":"28bf33ddc59e551445ac999cc41b841a4c49b4d8"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"93dd86a2032841483d3ed7fc7db4d78de2fea38e","unresolved":true,"context_lines":[{"line_number":73,"context_line":"        uuids.append(node_uuid)"},{"line_number":74,"context_line":"        found.append(fn)"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    if uuids:"},{"line_number":77,"context_line":"        # Any identities we found must be consistent, or we fail"},{"line_number":78,"context_line":"        first \u003d uuids[0]"},{"line_number":79,"context_line":"        for i, (node_uuid, fn) in enumerate(zip(uuids, found)):"}],"source_content_type":"text/x-python","patch_set":3,"id":"78e72e97_a88c020e","line":76,"in_reply_to":"5bc9dd62_dff8ea83","updated":"2023-01-13 00:21:44.000000000","message":"I like this 😊","commit_id":"28bf33ddc59e551445ac999cc41b841a4c49b4d8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a95c576892997ec1c38434aaeb217c75f321675e","unresolved":true,"context_lines":[{"line_number":73,"context_line":"        uuids.append(node_uuid)"},{"line_number":74,"context_line":"        found.append(fn)"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    if uuids:"},{"line_number":77,"context_line":"        # Any identities we found must be consistent, or we fail"},{"line_number":78,"context_line":"        first \u003d uuids[0]"},{"line_number":79,"context_line":"        for i, (node_uuid, fn) in enumerate(zip(uuids, found)):"}],"source_content_type":"text/x-python","patch_set":3,"id":"46e9d054_a80d2347","line":76,"in_reply_to":"78e72e97_a88c020e","updated":"2023-01-13 17:27:53.000000000","message":"The reason I didn\u0027t is so that the log can contain the offending file. So you could have the same thing in three files, which is all good. But if you have one that disagrees, it\u0027s nice to tell them which one it is instead of making them eye-diff them at the command line manually.","commit_id":"28bf33ddc59e551445ac999cc41b841a4c49b4d8"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4ee9eca9173be3f7d042c5f764647268561eaa54","unresolved":true,"context_lines":[{"line_number":35,"context_line":"    # Try to create the identity file and write our uuid into it. Fail"},{"line_number":36,"context_line":"    # if the file exists (since it shouldn\u0027t if we made it here)."},{"line_number":37,"context_line":"    try:"},{"line_number":38,"context_line":"        open(fn, \u0027x\u0027).write(node_uuid)"},{"line_number":39,"context_line":"    except FileExistsError:"},{"line_number":40,"context_line":"        # If the file exists, we must either fail or re-survey all the"},{"line_number":41,"context_line":"        # potential files. If we just read and return it, it could be"}],"source_content_type":"text/x-python","patch_set":7,"id":"8ab39fd9_4557f3d8","line":38,"updated":"2023-01-24 01:13:31.000000000","message":"nit you could have used the with: approch ehre too","commit_id":"3b33b0938ea4f5b00537298bc7fcb531fb9ec811"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"070a54048e69357c52883cdf2b97691dd4f1b193","unresolved":false,"context_lines":[{"line_number":35,"context_line":"    # Try to create the identity file and write our uuid into it. Fail"},{"line_number":36,"context_line":"    # if the file exists (since it shouldn\u0027t if we made it here)."},{"line_number":37,"context_line":"    try:"},{"line_number":38,"context_line":"        open(fn, \u0027x\u0027).write(node_uuid)"},{"line_number":39,"context_line":"    except FileExistsError:"},{"line_number":40,"context_line":"        # If the file exists, we must either fail or re-survey all the"},{"line_number":41,"context_line":"        # potential files. If we just read and return it, it could be"}],"source_content_type":"text/x-python","patch_set":7,"id":"a1f7a614_3ae1f7d4","line":38,"in_reply_to":"2122e4c2_05032656","updated":"2023-01-26 20:53:11.000000000","message":"Ack","commit_id":"3b33b0938ea4f5b00537298bc7fcb531fb9ec811"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8c38456dd21f04e4f55570b2adfbfd33a4ff0f65","unresolved":true,"context_lines":[{"line_number":35,"context_line":"    # Try to create the identity file and write our uuid into it. Fail"},{"line_number":36,"context_line":"    # if the file exists (since it shouldn\u0027t if we made it here)."},{"line_number":37,"context_line":"    try:"},{"line_number":38,"context_line":"        open(fn, \u0027x\u0027).write(node_uuid)"},{"line_number":39,"context_line":"    except FileExistsError:"},{"line_number":40,"context_line":"        # If the file exists, we must either fail or re-survey all the"},{"line_number":41,"context_line":"        # potential files. If we just read and return it, it could be"}],"source_content_type":"text/x-python","patch_set":7,"id":"2122e4c2_05032656","line":38,"in_reply_to":"8ab39fd9_4557f3d8","updated":"2023-01-26 18:49:42.000000000","message":"I will hit this in a FUP or a respin, yep.","commit_id":"3b33b0938ea4f5b00537298bc7fcb531fb9ec811"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4ee9eca9173be3f7d042c5f764647268561eaa54","unresolved":true,"context_lines":[{"line_number":64,"context_line":"            # to a legit UUID once we strip the whitespace."},{"line_number":65,"context_line":"            with open(fn) as f:"},{"line_number":66,"context_line":"                content \u003d f.read(40)"},{"line_number":67,"context_line":"            node_uuid \u003d str(uuid.UUID(content.strip()))"},{"line_number":68,"context_line":"        except FileNotFoundError:"},{"line_number":69,"context_line":"            continue"},{"line_number":70,"context_line":"        except ValueError:"}],"source_content_type":"text/x-python","patch_set":7,"id":"ff2a6f5e_2893e812","line":67,"updated":"2023-01-24 01:13:31.000000000","message":"this is both somehow more fragile (reading 40) and robust(stripign white space)\nthen i was expecting.\n\nhonestly i was expecting just a  single readline or reading the entire file\nthen striping the content.\n\nthis is proably fine just not how i would have done it.\n\nwith that said fi this file happens to be a 100GB db backup or somthing your way will still only read 40 bytes/charaters so its proably safer.","commit_id":"3b33b0938ea4f5b00537298bc7fcb531fb9ec811"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"896cc717bd9a3562e29a903644988ef92c9e6023","unresolved":false,"context_lines":[{"line_number":64,"context_line":"            # to a legit UUID once we strip the whitespace."},{"line_number":65,"context_line":"            with open(fn) as f:"},{"line_number":66,"context_line":"                content \u003d f.read(40)"},{"line_number":67,"context_line":"            node_uuid \u003d str(uuid.UUID(content.strip()))"},{"line_number":68,"context_line":"        except FileNotFoundError:"},{"line_number":69,"context_line":"            continue"},{"line_number":70,"context_line":"        except ValueError:"}],"source_content_type":"text/x-python","patch_set":7,"id":"559eb1da_2f100a70","line":67,"in_reply_to":"ff2a6f5e_2893e812","updated":"2023-01-26 15:42:35.000000000","message":"Ack","commit_id":"3b33b0938ea4f5b00537298bc7fcb531fb9ec811"}]}
