)]}'
{"nova/objects/instance.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"591c2b7863421bab8e60396cc0227bc8ce85c40f","unresolved":false,"context_lines":[{"line_number":747,"context_line":"            if obj is not None:"},{"line_number":748,"context_line":"                value \u003d jsonutils.dumps(obj.obj_to_primitive())"},{"line_number":749,"context_line":"            self._extra_values_to_save[field] \u003d value"},{"line_number":750,"context_line":"            self.obj_reset_changes([field])"},{"line_number":751,"context_line":""},{"line_number":752,"context_line":"    # TODO(stephenfin): Remove the \u0027admin_state_reset\u0027 field in version 3.0 of"},{"line_number":753,"context_line":"    # the object"}],"source_content_type":"text/x-python","patch_set":15,"id":"bf51134e_e8ec2abc","line":750,"range":{"start_line":750,"start_character":12,"end_line":750,"end_character":43},"updated":"2020-06-29 14:40:14.000000000","message":"So, this will not propagate to the sub-objects. If you want to do this, then it needs to have recursive\u003dTrue, but all this will do is reset the tracking in our object, but a subsequent call to obj_what_changed() will delegate to the sub-object and still report dirtiness:\n\nhttps://github.com/openstack/oslo.versionedobjects/blob/master/oslo_versionedobjects/base.py#L616\n\nSee my comment in the test.","commit_id":"9de97ec8f2b71a639a8c3fb9618af9f5b86107aa"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"89f3c209e9658b8dac54f4949260f0e999af34e6","unresolved":false,"context_lines":[{"line_number":747,"context_line":"            if obj is not None:"},{"line_number":748,"context_line":"                value \u003d jsonutils.dumps(obj.obj_to_primitive())"},{"line_number":749,"context_line":"            self._extra_values_to_save[field] \u003d value"},{"line_number":750,"context_line":"            self.obj_reset_changes([field])"},{"line_number":751,"context_line":""},{"line_number":752,"context_line":"    # TODO(stephenfin): Remove the \u0027admin_state_reset\u0027 field in version 3.0 of"},{"line_number":753,"context_line":"    # the object"}],"source_content_type":"text/x-python","patch_set":15,"id":"bf51134e_d23e24b8","line":750,"range":{"start_line":750,"start_character":12,"end_line":750,"end_character":43},"in_reply_to":"bf51134e_e8ec2abc","updated":"2020-06-30 11:08:21.000000000","message":"Done","commit_id":"9de97ec8f2b71a639a8c3fb9618af9f5b86107aa"}],"nova/tests/functional/regressions/test_bug_1843708.py":[{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"8b019bafee8c9f209f468bd589cd31c455ac7cda","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        fake_notifier.wait_for_versioned_notifications(\u0027instance.rebuild.end\u0027)"},{"line_number":64,"context_line":"        self._wait_for_state_change(server, \u0027ACTIVE\u0027)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        # Check keypairs changed"},{"line_number":67,"context_line":"        instance \u003d objects.Instance.get_by_uuid("},{"line_number":68,"context_line":"            ctxt, server[\u0027id\u0027], expected_attrs\u003d[\u0027keypairs\u0027])"},{"line_number":69,"context_line":"        self.assertEqual("},{"line_number":70,"context_line":"            keypair2[\u0027public_key\u0027], instance.keypairs[0].public_key)"}],"source_content_type":"text/x-python","patch_set":19,"id":"bf51134e_b5fcf03f","line":70,"range":{"start_line":66,"start_character":0,"end_line":70,"end_character":68},"updated":"2020-07-22 14:43:58.000000000","message":"FWIW we normally land regression tests ahead of the actual fix to show the broken behaviour. For example here we\u0027d assert we still see keypair1 in the instance with a FIXME comment detailing the issue. We would then fix the issue and update the test in this change.\n\nNothing to block this change on but just a note for next time *if* you wanted to land the test like that.","commit_id":"086796021b189c3ac64805ed8f6bde833906d284"}],"nova/tests/unit/fake_instance.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c0d1b5aebc504feef8f1369e468867aab224ff6d","unresolved":false,"context_lines":[{"line_number":140,"context_line":"    inst.new_flavor \u003d None"},{"line_number":141,"context_line":"    inst.resources \u003d None"},{"line_number":142,"context_line":"    inst.migration_context \u003d None"},{"line_number":143,"context_line":"    inst.obj_reset_changes(recursive\u003dTrue)"},{"line_number":144,"context_line":"    return inst"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"bf51134e_08bebefd","line":143,"updated":"2020-06-29 14:16:32.000000000","message":"fwiw, we could also have added\n\n  inst.keypairs.obj_reset_changes()\n\nhere. Might be less invasive, though this seems to be functionally identical.","commit_id":"9de97ec8f2b71a639a8c3fb9618af9f5b86107aa"}],"nova/tests/unit/objects/test_instance.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"7214723657404241454ee57e811269bd30e94536","unresolved":false,"context_lines":[{"line_number":691,"context_line":"        expected_vals \u003d {\u0027numa_topology\u0027: None,"},{"line_number":692,"context_line":"                         \u0027migration_context\u0027: None,"},{"line_number":693,"context_line":"                         \u0027vcpu_model\u0027: json_vcpu_model,"},{"line_number":694,"context_line":"                         \u0027keypairs\u0027: json_keypairs}"},{"line_number":695,"context_line":"        mock_update.assert_called_once_with(self.context, inst.uuid,"},{"line_number":696,"context_line":"                                            expected_vals)"},{"line_number":697,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3fa7e38b_ca40aaa5","line":694,"updated":"2019-11-13 18:38:05.000000000","message":"This ensures that the _save_extra_generic() gets called for the keypairs, but that\u0027s not enough. If you make this change:\n\n diff --git a/nova/tests/unit/objects/test_instance.py b/nova/tests/unit/objects/test_instance.py\n index c9ccdff959..3d0308b2c0 100644\n --- a/nova/tests/unit/objects/test_instance.py\n +++ b/nova/tests/unit/objects/test_instance.py\n @@ -483,6 +483,8 @@ class _TestInstanceObject(object):\n          inst.vm_state \u003d \u0027meow\u0027\n          inst.task_state \u003d \u0027wuff\u0027\n          inst.user_data \u003d \u0027new\u0027\n +        inst.keypairs \u003d objects.KeyPairList(objects\u003d[\n +            objects.KeyPair(name\u003d\u0027foo\u0027)])\n          save_kwargs.pop(\u0027context\u0027, None)\n          inst.save(**save_kwargs)\n          self.assertEqual(\u0027newhost\u0027, inst.host)\n\nYou will see that the keypairs attribute stays dirty after the save() because implicit marking of nested fields as clean isn\u0027t always straightforward.\n\nI think the proper thing to do is keep the _save_keypairs() handler, call the generic save handler, and then self.obj_reset_changes(\u0027keypairs\u0027). See _save_security_groups() or _save_flavor() for examples.","commit_id":"6d314aeb74819cd71da7407b226675e48d8a2fdb"},{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"0c13d18a764dab4053f9b2c6ac664ca7ce78d262","unresolved":false,"context_lines":[{"line_number":691,"context_line":"        expected_vals \u003d {\u0027numa_topology\u0027: None,"},{"line_number":692,"context_line":"                         \u0027migration_context\u0027: None,"},{"line_number":693,"context_line":"                         \u0027vcpu_model\u0027: json_vcpu_model,"},{"line_number":694,"context_line":"                         \u0027keypairs\u0027: json_keypairs}"},{"line_number":695,"context_line":"        mock_update.assert_called_once_with(self.context, inst.uuid,"},{"line_number":696,"context_line":"                                            expected_vals)"},{"line_number":697,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3fa7e38b_19acdb9d","line":694,"in_reply_to":"3fa7e38b_ca40aaa5","updated":"2019-11-13 23:16:13.000000000","message":"Done","commit_id":"6d314aeb74819cd71da7407b226675e48d8a2fdb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"591c2b7863421bab8e60396cc0227bc8ce85c40f","unresolved":false,"context_lines":[{"line_number":686,"context_line":"        inst.vcpu_model \u003d test_vcpu_model.fake_vcpumodel"},{"line_number":687,"context_line":"        test_keypairs \u003d objects.KeyPairList("},{"line_number":688,"context_line":"            objects\u003d[objects.KeyPair(name\u003d\u0027foo\u0027)])"},{"line_number":689,"context_line":"        test_keypairs.obj_reset_changes(recursive\u003dTrue)"},{"line_number":690,"context_line":"        inst.keypairs \u003d test_keypairs"},{"line_number":691,"context_line":"        # Check changed fields in the instance object."},{"line_number":692,"context_line":"        self.assertEqual("}],"source_content_type":"text/x-python","patch_set":15,"id":"bf51134e_c82966e6","line":689,"updated":"2020-06-29 14:40:14.000000000","message":"This makes no sense. You\u0027re resetting changes on a thing you want to be save()d. The only reason you\u0027re noticing this in the save handler is that you\u0027re setting the object on the instance after this, which also tickle\u0027s the parent\u0027s dirty flag for the object.\n\nIf you made this something more realistic, where you modify instance.keypairs (like to add a new one or change the keypairs[0] object) instead of setting it directly on the top-level object, you\u0027d fail your test on L708 because your reset in the instance object doesn\u0027t reset the flag on save(). For example:\n\n inst.keypairs \u003d objects.KeyPairList([])\n inst.keypairs.obj_reset_changes(recurisive\u003dTrue)\n inst.keypairs.objects.append(objects.KeyPair(name\u003d\u0027foo\u0027))\n inst.save()\n\nThis is a much more common case of updating keypairs during rebuild, which would leave inst.keypairs dirty, and fail your assertion on L708.","commit_id":"9de97ec8f2b71a639a8c3fb9618af9f5b86107aa"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"89f3c209e9658b8dac54f4949260f0e999af34e6","unresolved":false,"context_lines":[{"line_number":686,"context_line":"        inst.vcpu_model \u003d test_vcpu_model.fake_vcpumodel"},{"line_number":687,"context_line":"        test_keypairs \u003d objects.KeyPairList("},{"line_number":688,"context_line":"            objects\u003d[objects.KeyPair(name\u003d\u0027foo\u0027)])"},{"line_number":689,"context_line":"        test_keypairs.obj_reset_changes(recursive\u003dTrue)"},{"line_number":690,"context_line":"        inst.keypairs \u003d test_keypairs"},{"line_number":691,"context_line":"        # Check changed fields in the instance object."},{"line_number":692,"context_line":"        self.assertEqual("}],"source_content_type":"text/x-python","patch_set":15,"id":"bf51134e_b23bb0c6","line":689,"in_reply_to":"bf51134e_c82966e6","updated":"2020-06-30 11:08:21.000000000","message":"Done","commit_id":"9de97ec8f2b71a639a8c3fb9618af9f5b86107aa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c8b5a858c4af9259c5d30faf936a71fd852b4191","unresolved":false,"context_lines":[{"line_number":705,"context_line":"        mock_update.assert_called_once_with(self.context, inst.uuid,"},{"line_number":706,"context_line":"                                            expected_vals)"},{"line_number":707,"context_line":"        # Verify that the record of changed fields has been cleared."},{"line_number":708,"context_line":"        self.assertFalse(len(inst.obj_what_changed()))"},{"line_number":709,"context_line":""},{"line_number":710,"context_line":"    @mock.patch.object(db, \u0027instance_get_by_uuid\u0027)"},{"line_number":711,"context_line":"    def test_get_deleted(self, mock_get):"}],"source_content_type":"text/x-python","patch_set":15,"id":"bf51134e_c8de866b","line":708,"range":{"start_line":708,"start_character":25,"end_line":708,"end_character":53},"updated":"2020-06-29 14:17:50.000000000","message":"This is not a boolean","commit_id":"9de97ec8f2b71a639a8c3fb9618af9f5b86107aa"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e66143f41d0c6dd027d7ec9068f1c7e19dcd9428","unresolved":false,"context_lines":[{"line_number":705,"context_line":"        mock_update.assert_called_once_with(self.context, inst.uuid,"},{"line_number":706,"context_line":"                                            expected_vals)"},{"line_number":707,"context_line":"        # Verify that the record of changed fields has been cleared."},{"line_number":708,"context_line":"        self.assertFalse(len(inst.obj_what_changed()))"},{"line_number":709,"context_line":""},{"line_number":710,"context_line":"    @mock.patch.object(db, \u0027instance_get_by_uuid\u0027)"},{"line_number":711,"context_line":"    def test_get_deleted(self, mock_get):"}],"source_content_type":"text/x-python","patch_set":15,"id":"bf51134e_081a3e15","line":708,"range":{"start_line":708,"start_character":25,"end_line":708,"end_character":53},"in_reply_to":"bf51134e_c8de866b","updated":"2020-06-29 14:21:57.000000000","message":"Yeah\n\n  self.assertEqual(0, len(inst.obj_what_changed()))\n\nwould be better, or even just\n\n  self.assertEqual(set(), inst.obj_what_changed())\n\n(once again, _why_ didn\u0027t Python core choose a different symbol for set notation :\u0027()","commit_id":"9de97ec8f2b71a639a8c3fb9618af9f5b86107aa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"63d955f64024cf4739a4077f43e4bcf4af26a3c1","unresolved":false,"context_lines":[{"line_number":701,"context_line":"            \u0027migration_context\u0027: None,"},{"line_number":702,"context_line":"            \u0027vcpu_model\u0027: json_vcpu_model,"},{"line_number":703,"context_line":"            \u0027keypairs\u0027: json_keypairs,"},{"line_number":704,"context_line":"        }"},{"line_number":705,"context_line":"        mock_update.assert_called_once_with(self.context, inst.uuid,"},{"line_number":706,"context_line":"                                            expected_vals)"},{"line_number":707,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"bf51134e_8020aff9","line":704,"updated":"2020-07-02 14:45:19.000000000","message":"This didn\u0027t need to be reformatted and just requires reviewers to eye-parse it to confirm that nothing changed.","commit_id":"6151091e69835db8eadad71a36ab892b09d8b1f7"}]}