)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7c585c01377e82fe1f38fa53d81bd5597f7aa6c5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"8a86fdc0_9730e672","updated":"2025-02-19 17:40:13.000000000","message":"+2 while we wait for zuul to complete","commit_id":"420050cf33dd58bc6b608540fc8f32b2f65ef274"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"0b43df064d794583230fd3cac3efa8b9fd287c77","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"210e0e5f_67d9b3d8","updated":"2025-02-19 16:02:32.000000000","message":"+2 with comments not worth holding the approval","commit_id":"420050cf33dd58bc6b608540fc8f32b2f65ef274"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"013ac128af95ccc1a9341cfc580373c21e5506b9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"a81f3e73_fd3a7f44","updated":"2025-02-20 15:28:36.000000000","message":"we agreed to merge it, thanks Dan for the help","commit_id":"420050cf33dd58bc6b608540fc8f32b2f65ef274"}],"nova/objects/instance.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7c585c01377e82fe1f38fa53d81bd5597f7aa6c5","unresolved":false,"context_lines":[{"line_number":1603,"context_line":"                db_inst_shells.values(),"},{"line_number":1604,"context_line":"                manual_joins\u003d[\u0027system_metadata\u0027])"},{"line_number":1605,"context_line":"            updated \u003d {i[\u0027uuid\u0027]: i for i in updates}"},{"line_number":1606,"context_line":"            for inst in [i for i in self if i.uuid in updated]:"},{"line_number":1607,"context_line":"                # Patch up our instances with system_metadata from the fill"},{"line_number":1608,"context_line":"                # operation"},{"line_number":1609,"context_line":"                inst.system_metadata \u003d utils.instance_sys_meta(updated)"}],"source_content_type":"text/x-python","patch_set":1,"id":"34faf0af_2eae710a","line":1606,"range":{"start_line":1606,"start_character":12,"end_line":1606,"end_character":63},"updated":"2025-02-19 17:40:13.000000000","message":"[i for i in self if i.uuid in updated]\n\nis the list of instances in self that are updated.\n\nThe we do a for over that list and update each instance system_metadata.\n\nI think this could be a generator expression instead of a list comprehension\n\n```suggestion\n            for inst in (i for i in self if i.uuid in updated):\n```\nsince we are only iterating over it once\nhttps://peps.python.org/pep-0289/\n\nwith that said i dont see a reason to refactor  the memory overhead in this case is pretty small given we are really jsut dealing with the pointer tothe insts obejcts not making a copy\n\nfilter while exactly for this uscase is also too fancy for something this trivial.\n\n```\n            for inst in filter(lambda i: i.uuid in updated, iter(self)):\n```\n\nim commenting on this becasue this line is a little dense at first glance\nbut the logic looks correct to me.","commit_id":"420050cf33dd58bc6b608540fc8f32b2f65ef274"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"0b0ef46008ac23979fdfe3990790df56171e0820","unresolved":false,"context_lines":[{"line_number":1603,"context_line":"                db_inst_shells.values(),"},{"line_number":1604,"context_line":"                manual_joins\u003d[\u0027system_metadata\u0027])"},{"line_number":1605,"context_line":"            updated \u003d {i[\u0027uuid\u0027]: i for i in updates}"},{"line_number":1606,"context_line":"            for inst in [i for i in self if i.uuid in updated]:"},{"line_number":1607,"context_line":"                # Patch up our instances with system_metadata from the fill"},{"line_number":1608,"context_line":"                # operation"},{"line_number":1609,"context_line":"                inst.system_metadata \u003d utils.instance_sys_meta(updated)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ef6e1558_7300e2eb","line":1606,"range":{"start_line":1606,"start_character":12,"end_line":1606,"end_character":63},"in_reply_to":"34faf0af_2eae710a","updated":"2025-02-19 17:42:20.000000000","message":"Ack I can make it a generator if I respin","commit_id":"420050cf33dd58bc6b608540fc8f32b2f65ef274"}],"nova/tests/unit/objects/test_instance.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7c585c01377e82fe1f38fa53d81bd5597f7aa6c5","unresolved":false,"context_lines":[{"line_number":2074,"context_line":""},{"line_number":2075,"context_line":"        # One instance has system_metadata populated *and* saved in the DB"},{"line_number":2076,"context_line":"        insts[1].system_metadata \u003d {\u0027BTTF1\u0027: \u00271955\u0027}"},{"line_number":2077,"context_line":"        insts[1].save()"},{"line_number":2078,"context_line":""},{"line_number":2079,"context_line":"        # One instance has system_metadata populated but dirty"},{"line_number":2080,"context_line":"        insts[2].system_metadata \u003d {\u0027bttf3\u0027: \u00271885\u0027}"}],"source_content_type":"text/x-python","patch_set":1,"id":"5341a1c9_5a5b4292","line":2077,"updated":"2025-02-19 17:40:13.000000000","message":"while this will work and is corret it feels like an off by one error to start from 1 :)\nyou are uisng index 0 for an instance with no system metadata lather however so this checks out.","commit_id":"420050cf33dd58bc6b608540fc8f32b2f65ef274"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"0b43df064d794583230fd3cac3efa8b9fd287c77","unresolved":false,"context_lines":[{"line_number":2081,"context_line":""},{"line_number":2082,"context_line":"        # Three do not have system_metadata populated"},{"line_number":2083,"context_line":"        self.assertEqual(3, len([i for i in insts"},{"line_number":2084,"context_line":"                                 if \u0027system_metadata\u0027 not in i]))"},{"line_number":2085,"context_line":""},{"line_number":2086,"context_line":"        insts.fill_metadata()"},{"line_number":2087,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"388d9341_02d03d27","line":2084,"updated":"2025-02-19 16:02:32.000000000","message":"++","commit_id":"420050cf33dd58bc6b608540fc8f32b2f65ef274"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"0b43df064d794583230fd3cac3efa8b9fd287c77","unresolved":true,"context_lines":[{"line_number":2092,"context_line":"        # Inst 2 should have not had its in-memory copy clobbered"},{"line_number":2093,"context_line":"        self.assertEqual({\u0027bttf3\u0027: \u00271885\u0027}, insts[2].system_metadata)"},{"line_number":2094,"context_line":""},{"line_number":2095,"context_line":"        # Inst 1 should have system_metadata loaded, but empty"},{"line_number":2096,"context_line":"        self.assertIn(\u0027system_metadata\u0027, insts[0])"},{"line_number":2097,"context_line":"        self.assertEqual({}, insts[0].system_metadata)"},{"line_number":2098,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"8e095bd0_17ecf206","line":2095,"range":{"start_line":2095,"start_character":10,"end_line":2095,"end_character":16},"updated":"2025-02-19 16:02:32.000000000","message":"nit: that\u0027s \u0027inst 0\u0027 (or the first instance in the list but given you said inst 2 for insts[2] above, the numbering is not consistent in the doc.","commit_id":"420050cf33dd58bc6b608540fc8f32b2f65ef274"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7c585c01377e82fe1f38fa53d81bd5597f7aa6c5","unresolved":true,"context_lines":[{"line_number":2092,"context_line":"        # Inst 2 should have not had its in-memory copy clobbered"},{"line_number":2093,"context_line":"        self.assertEqual({\u0027bttf3\u0027: \u00271885\u0027}, insts[2].system_metadata)"},{"line_number":2094,"context_line":""},{"line_number":2095,"context_line":"        # Inst 1 should have system_metadata loaded, but empty"},{"line_number":2096,"context_line":"        self.assertIn(\u0027system_metadata\u0027, insts[0])"},{"line_number":2097,"context_line":"        self.assertEqual({}, insts[0].system_metadata)"},{"line_number":2098,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"ec675df4_9b3d6823","line":2095,"range":{"start_line":2095,"start_character":10,"end_line":2095,"end_character":16},"in_reply_to":"72dfa60e_59eae874","updated":"2025-02-19 17:40:13.000000000","message":"this feel like something we should not respine for on it s own but i agree if we dont proceed with this as is it would be nice to align on index or cardonality but i dont have a prefernce.","commit_id":"420050cf33dd58bc6b608540fc8f32b2f65ef274"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4304532a7c1edd85cbbf69c6219c883953de99e7","unresolved":true,"context_lines":[{"line_number":2092,"context_line":"        # Inst 2 should have not had its in-memory copy clobbered"},{"line_number":2093,"context_line":"        self.assertEqual({\u0027bttf3\u0027: \u00271885\u0027}, insts[2].system_metadata)"},{"line_number":2094,"context_line":""},{"line_number":2095,"context_line":"        # Inst 1 should have system_metadata loaded, but empty"},{"line_number":2096,"context_line":"        self.assertIn(\u0027system_metadata\u0027, insts[0])"},{"line_number":2097,"context_line":"        self.assertEqual({}, insts[0].system_metadata)"},{"line_number":2098,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"72dfa60e_59eae874","line":2095,"range":{"start_line":2095,"start_character":10,"end_line":2095,"end_character":16},"in_reply_to":"8e095bd0_17ecf206","updated":"2025-02-19 16:04:56.000000000","message":"I was referring to instances by order not index, but I can change this if I have to respin.","commit_id":"420050cf33dd58bc6b608540fc8f32b2f65ef274"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"0b0ef46008ac23979fdfe3990790df56171e0820","unresolved":false,"context_lines":[{"line_number":2092,"context_line":"        # Inst 2 should have not had its in-memory copy clobbered"},{"line_number":2093,"context_line":"        self.assertEqual({\u0027bttf3\u0027: \u00271885\u0027}, insts[2].system_metadata)"},{"line_number":2094,"context_line":""},{"line_number":2095,"context_line":"        # Inst 1 should have system_metadata loaded, but empty"},{"line_number":2096,"context_line":"        self.assertIn(\u0027system_metadata\u0027, insts[0])"},{"line_number":2097,"context_line":"        self.assertEqual({}, insts[0].system_metadata)"},{"line_number":2098,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"affb499b_303d4e3f","line":2095,"range":{"start_line":2095,"start_character":10,"end_line":2095,"end_character":16},"in_reply_to":"ec675df4_9b3d6823","updated":"2025-02-19 17:42:20.000000000","message":"Acknowledged","commit_id":"420050cf33dd58bc6b608540fc8f32b2f65ef274"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"0b43df064d794583230fd3cac3efa8b9fd287c77","unresolved":true,"context_lines":[{"line_number":2095,"context_line":"        # Inst 1 should have system_metadata loaded, but empty"},{"line_number":2096,"context_line":"        self.assertIn(\u0027system_metadata\u0027, insts[0])"},{"line_number":2097,"context_line":"        self.assertEqual({}, insts[0].system_metadata)"},{"line_number":2098,"context_line":""},{"line_number":2099,"context_line":"    def test_fill_metadata_nop(self):"},{"line_number":2100,"context_line":"        insts \u003d objects.InstanceList([objects.Instance(uuid\u003duuids.inst,"},{"line_number":2101,"context_line":"                                                       system_metadata\u003d{})])"}],"source_content_type":"text/x-python","patch_set":1,"id":"e441ed36_a83bb0f8","line":2098,"updated":"2025-02-19 16:02:32.000000000","message":"nit: we could also check insts[3] and insts[4] but I don\u0027t see the need.","commit_id":"420050cf33dd58bc6b608540fc8f32b2f65ef274"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7c585c01377e82fe1f38fa53d81bd5597f7aa6c5","unresolved":false,"context_lines":[{"line_number":2095,"context_line":"        # Inst 1 should have system_metadata loaded, but empty"},{"line_number":2096,"context_line":"        self.assertIn(\u0027system_metadata\u0027, insts[0])"},{"line_number":2097,"context_line":"        self.assertEqual({}, insts[0].system_metadata)"},{"line_number":2098,"context_line":""},{"line_number":2099,"context_line":"    def test_fill_metadata_nop(self):"},{"line_number":2100,"context_line":"        insts \u003d objects.InstanceList([objects.Instance(uuid\u003duuids.inst,"},{"line_number":2101,"context_line":"                                                       system_metadata\u003d{})])"}],"source_content_type":"text/x-python","patch_set":1,"id":"2a906a33_b94248a5","line":2098,"in_reply_to":"e441ed36_a83bb0f8","updated":"2025-02-19 17:40:13.000000000","message":"Acknowledged","commit_id":"420050cf33dd58bc6b608540fc8f32b2f65ef274"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"0b43df064d794583230fd3cac3efa8b9fd287c77","unresolved":false,"context_lines":[{"line_number":2101,"context_line":"                                                       system_metadata\u003d{})])"},{"line_number":2102,"context_line":"        # This would fail if it tries to actually load anything"},{"line_number":2103,"context_line":"        # because context is None"},{"line_number":2104,"context_line":"        insts.fill_metadata()"},{"line_number":2105,"context_line":""},{"line_number":2106,"context_line":""},{"line_number":2107,"context_line":"class TestRemoteInstanceListObject(test_objects._RemoteTest,"}],"source_content_type":"text/x-python","patch_set":1,"id":"d44d753a_b2003117","line":2104,"updated":"2025-02-19 16:02:32.000000000","message":"I like this negative test.","commit_id":"420050cf33dd58bc6b608540fc8f32b2f65ef274"}]}