)]}'
{"nova/compute/api.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d030cadf3245050bfa8bac4139752ebad29ed849","unresolved":false,"context_lines":[{"line_number":977,"context_line":"                instance.uuid \u003d instance_uuid"},{"line_number":978,"context_line":"                instance.update(base_options)"},{"line_number":979,"context_line":"                if key_pair:"},{"line_number":980,"context_line":"                    instance.keypairs \u003d objects.KeyPairList(objects\u003d[key_pair])"},{"line_number":981,"context_line":"                instance \u003d self.create_db_entry_for_new_instance(context,"},{"line_number":982,"context_line":"                        instance_type, boot_meta, instance, security_groups,"},{"line_number":983,"context_line":"                        block_device_mapping, num_instances, i,"}],"source_content_type":"text/x-python","patch_set":5,"id":"dab17558_bdbcf3bb","line":980,"updated":"2016-05-10 14:33:36.000000000","message":"Oops, we should create with an empty list here instead of None.","commit_id":"42f94e813c4f5746e98205a6261311c5f884c56c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3738efb7196a88fdb9bb06e1d82df27dfa389eab","unresolved":false,"context_lines":[{"line_number":941,"context_line":"                info_cache\u003dinfo_cache,"},{"line_number":942,"context_line":"                security_groups\u003dsecurity_groups,"},{"line_number":943,"context_line":"                config_drive\u003dbool_config_drive,"},{"line_number":944,"context_line":"                key_name\u003dbase_options[\u0027key_name\u0027],"},{"line_number":945,"context_line":"                locked_by\u003dNone)"},{"line_number":946,"context_line":"        build_request.create()"},{"line_number":947,"context_line":"        return build_request"}],"source_content_type":"text/x-python","patch_set":7,"id":"dab17558_ab8bfbc1","line":944,"range":{"start_line":944,"start_character":16,"end_line":944,"end_character":50},"updated":"2016-05-10 18:59:36.000000000","message":"Hmm, I guess this is OK since the build request is just used for representing the instance in the API if it isn\u0027t scheduled somewhere, and storing keypairs in the instance object doesn\u0027t change that.","commit_id":"0ab6f95fb6b9b4732d6f6bcada3ff13eb2e0288f"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"9f4053ac6c44058203e8a09bdea392886edaf0a8","unresolved":false,"context_lines":[{"line_number":941,"context_line":"                info_cache\u003dinfo_cache,"},{"line_number":942,"context_line":"                security_groups\u003dsecurity_groups,"},{"line_number":943,"context_line":"                config_drive\u003dbool_config_drive,"},{"line_number":944,"context_line":"                key_name\u003dbase_options[\u0027key_name\u0027],"},{"line_number":945,"context_line":"                locked_by\u003dNone)"},{"line_number":946,"context_line":"        build_request.create()"},{"line_number":947,"context_line":"        return build_request"}],"source_content_type":"text/x-python","patch_set":7,"id":"dab17558_d150b2d6","line":944,"range":{"start_line":944,"start_character":16,"end_line":944,"end_character":50},"in_reply_to":"dab17558_ab8bfbc1","updated":"2016-05-10 19:39:57.000000000","message":"Also, this is unused now and will be going away soon. https://review.openstack.org/#/c/305953/5/nova/objects/build_request.py\n\nAll that matter is the instance and instance_uuid passed on lines 928-929.","commit_id":"0ab6f95fb6b9b4732d6f6bcada3ff13eb2e0288f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d39e022005979002dd9bba9444b8569041d59871","unresolved":false,"context_lines":[{"line_number":975,"context_line":"                # Create an instance object, but do not store in db yet."},{"line_number":976,"context_line":"                instance \u003d objects.Instance(context\u003dcontext)"},{"line_number":977,"context_line":"                instance.uuid \u003d instance_uuid"},{"line_number":978,"context_line":"                instance.update(base_options)"},{"line_number":979,"context_line":"                instance.keypairs \u003d objects.KeyPairList(objects\u003d[])"},{"line_number":980,"context_line":"                if key_pair:"},{"line_number":981,"context_line":"                    instance.keypairs.objects.append(key_pair)"}],"source_content_type":"text/x-python","patch_set":7,"id":"dab17558_8b0b9729","line":978,"range":{"start_line":978,"start_character":16,"end_line":978,"end_character":45},"updated":"2016-05-10 19:04:47.000000000","message":"OK, so here we\u0027ll be setting key_name and key_data on the instance object, so we still have that information, and now it will just be duplicated in the instances table and the instance_extra table, until such time that we can drop the key_name/data fields from the instance object in v3.0 of that object, and get those details from the instance.keypairs[0], right?","commit_id":"0ab6f95fb6b9b4732d6f6bcada3ff13eb2e0288f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"fb82cb73e59f3c72bbf38d55ada80f6eff3c12c6","unresolved":false,"context_lines":[{"line_number":975,"context_line":"                # Create an instance object, but do not store in db yet."},{"line_number":976,"context_line":"                instance \u003d objects.Instance(context\u003dcontext)"},{"line_number":977,"context_line":"                instance.uuid \u003d instance_uuid"},{"line_number":978,"context_line":"                instance.update(base_options)"},{"line_number":979,"context_line":"                instance.keypairs \u003d objects.KeyPairList(objects\u003d[])"},{"line_number":980,"context_line":"                if key_pair:"},{"line_number":981,"context_line":"                    instance.keypairs.objects.append(key_pair)"}],"source_content_type":"text/x-python","patch_set":7,"id":"dab17558_8ad0a820","line":978,"range":{"start_line":978,"start_character":16,"end_line":978,"end_character":45},"in_reply_to":"dab17558_8b0b9729","updated":"2016-05-10 23:24:16.000000000","message":"Yar.","commit_id":"0ab6f95fb6b9b4732d6f6bcada3ff13eb2e0288f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"93316c73d247d030881bc260b20fbdd5ba61f5a3","unresolved":false,"context_lines":[{"line_number":978,"context_line":"                instance.update(base_options)"},{"line_number":979,"context_line":"                instance.keypairs \u003d objects.KeyPairList(objects\u003d[])"},{"line_number":980,"context_line":"                if key_pair:"},{"line_number":981,"context_line":"                    instance.keypairs.objects.append(key_pair)"},{"line_number":982,"context_line":"                instance \u003d self.create_db_entry_for_new_instance(context,"},{"line_number":983,"context_line":"                        instance_type, boot_meta, instance, security_groups,"},{"line_number":984,"context_line":"                        block_device_mapping, num_instances, i,"}],"source_content_type":"text/x-python","patch_set":7,"id":"dab17558_ab277bc8","line":981,"updated":"2016-05-10 19:03:08.000000000","message":"As far as I can tell, we don\u0027t test this since the unit tests are all passing None back.","commit_id":"0ab6f95fb6b9b4732d6f6bcada3ff13eb2e0288f"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d11409ae45d36728240a8e9b319ef61398324122","unresolved":false,"context_lines":[{"line_number":978,"context_line":"                instance.update(base_options)"},{"line_number":979,"context_line":"                instance.keypairs \u003d objects.KeyPairList(objects\u003d[])"},{"line_number":980,"context_line":"                if key_pair:"},{"line_number":981,"context_line":"                    instance.keypairs.objects.append(key_pair)"},{"line_number":982,"context_line":"                instance \u003d self.create_db_entry_for_new_instance(context,"},{"line_number":983,"context_line":"                        instance_type, boot_meta, instance, security_groups,"},{"line_number":984,"context_line":"                        block_device_mapping, num_instances, i,"}],"source_content_type":"text/x-python","patch_set":7,"id":"dab17558_11807abf","line":981,"updated":"2016-05-10 19:48:04.000000000","message":"I double-checked the test_compute_api.py file and @mriedem is indeed correct. We always pass key_name as None to the _verify_blah_build_request() function and thus always pass back a key_pair of None. :(","commit_id":"0ab6f95fb6b9b4732d6f6bcada3ff13eb2e0288f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"408fed49a3d9fbb45e2c5a3dc562c6dcf0bd8682","unresolved":false,"context_lines":[{"line_number":978,"context_line":"                instance.update(base_options)"},{"line_number":979,"context_line":"                instance.keypairs \u003d objects.KeyPairList(objects\u003d[])"},{"line_number":980,"context_line":"                if key_pair:"},{"line_number":981,"context_line":"                    instance.keypairs.objects.append(key_pair)"},{"line_number":982,"context_line":"                instance \u003d self.create_db_entry_for_new_instance(context,"},{"line_number":983,"context_line":"                        instance_type, boot_meta, instance, security_groups,"},{"line_number":984,"context_line":"                        block_device_mapping, num_instances, i,"}],"source_content_type":"text/x-python","patch_set":7,"id":"dab17558_b18b6ec2","line":981,"in_reply_to":"dab17558_11807abf","updated":"2016-05-10 19:51:04.000000000","message":"We\u0027re just passing None to a mock, so it\u0027s not testing anything less :)\n\nAny test that actually uses a key pair that doesn\u0027t mock out the critical parts of the build process will do the right thing :)","commit_id":"0ab6f95fb6b9b4732d6f6bcada3ff13eb2e0288f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e27e573fe28c44ee2623b3ec0801c0082043ff37","unresolved":false,"context_lines":[{"line_number":978,"context_line":"                instance.update(base_options)"},{"line_number":979,"context_line":"                instance.keypairs \u003d objects.KeyPairList(objects\u003d[])"},{"line_number":980,"context_line":"                if key_pair:"},{"line_number":981,"context_line":"                    instance.keypairs.objects.append(key_pair)"},{"line_number":982,"context_line":"                instance \u003d self.create_db_entry_for_new_instance(context,"},{"line_number":983,"context_line":"                        instance_type, boot_meta, instance, security_groups,"},{"line_number":984,"context_line":"                        block_device_mapping, num_instances, i,"}],"source_content_type":"text/x-python","patch_set":8,"id":"dab17558_2df6f6f8","line":981,"updated":"2016-05-10 23:57:39.000000000","message":"I still feel a bit weird about not having an explicit unit test for this. I pulled this change down and added an exception in this block and nova.tests.unit.compute.test_compute_api didn\u0027t fail, but some functional tests did fail, however, they were cloudpipe functional tests, which is eventually going to be deleted.\n\nnova.tests.unit.compute.test_compute didn\u0027t fail either.\n\nSo I\u0027m not sure what was catching a failure here in the previous patch set, but I\u0027m a bit uneasy about not having something in test_compute_api for this.","commit_id":"b067ecdd9afaa534422ac07da815997a15e2e57e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9ea319985ca555135c8f1e6218bf12be2cb8b849","unresolved":false,"context_lines":[{"line_number":978,"context_line":"                instance.update(base_options)"},{"line_number":979,"context_line":"                instance.keypairs \u003d objects.KeyPairList(objects\u003d[])"},{"line_number":980,"context_line":"                if key_pair:"},{"line_number":981,"context_line":"                    instance.keypairs.objects.append(key_pair)"},{"line_number":982,"context_line":"                instance \u003d self.create_db_entry_for_new_instance(context,"},{"line_number":983,"context_line":"                        instance_type, boot_meta, instance, security_groups,"},{"line_number":984,"context_line":"                        block_device_mapping, num_instances, i,"}],"source_content_type":"text/x-python","patch_set":8,"id":"dab17558_6aa43707","line":981,"in_reply_to":"dab17558_2df6f6f8","updated":"2016-05-11 17:04:34.000000000","message":"And I continue to maintain that the unit tests for this stuff mock all of this out and it doesn\u0027t matter at all anyway. The functional tests actually do test this in a meaningful way.\n\nBut, I\u0027ll add a mocked-out-of-its-mind test to mirror the others that test very little to make it look like I\u0027m clean here. :)","commit_id":"b067ecdd9afaa534422ac07da815997a15e2e57e"}],"nova/objects/instance.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e27e573fe28c44ee2623b3ec0801c0082043ff37","unresolved":false,"context_lines":[{"line_number":466,"context_line":"            }"},{"line_number":467,"context_line":"            updates[\u0027extra\u0027][\u0027flavor\u0027] \u003d jsonutils.dumps(flavor_info)"},{"line_number":468,"context_line":"        keypairs \u003d updates.pop(\u0027keypairs\u0027, None)"},{"line_number":469,"context_line":"        if keypairs is not None:"},{"line_number":470,"context_line":"            expected_attrs.append(\u0027keypairs\u0027)"},{"line_number":471,"context_line":"            updates[\u0027extra\u0027][\u0027keypairs\u0027] \u003d jsonutils.dumps("},{"line_number":472,"context_line":"                keypairs.obj_to_primitive())"}],"source_content_type":"text/x-python","patch_set":8,"id":"dab17558_6a946442","line":469,"updated":"2016-05-10 23:57:39.000000000","message":"Ah because we init the instance in the compute API with an empty keypairs list.\n\nUnit test for this?\n\nAlternatively you could just not set instance.keypairs if there isn\u0027t one during the create.","commit_id":"b067ecdd9afaa534422ac07da815997a15e2e57e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9ea319985ca555135c8f1e6218bf12be2cb8b849","unresolved":false,"context_lines":[{"line_number":466,"context_line":"            }"},{"line_number":467,"context_line":"            updates[\u0027extra\u0027][\u0027flavor\u0027] \u003d jsonutils.dumps(flavor_info)"},{"line_number":468,"context_line":"        keypairs \u003d updates.pop(\u0027keypairs\u0027, None)"},{"line_number":469,"context_line":"        if keypairs is not None:"},{"line_number":470,"context_line":"            expected_attrs.append(\u0027keypairs\u0027)"},{"line_number":471,"context_line":"            updates[\u0027extra\u0027][\u0027keypairs\u0027] \u003d jsonutils.dumps("},{"line_number":472,"context_line":"                keypairs.obj_to_primitive())"}],"source_content_type":"text/x-python","patch_set":8,"id":"dab17558_6a56f706","line":469,"in_reply_to":"dab17558_6a946442","updated":"2016-05-11 17:04:34.000000000","message":"No, we can\u0027t do that. See the previous review comments.","commit_id":"b067ecdd9afaa534422ac07da815997a15e2e57e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b37f70423751b61f9482262c9d5540eb515c6759","unresolved":false,"context_lines":[{"line_number":466,"context_line":"            }"},{"line_number":467,"context_line":"            updates[\u0027extra\u0027][\u0027flavor\u0027] \u003d jsonutils.dumps(flavor_info)"},{"line_number":468,"context_line":"        keypairs \u003d updates.pop(\u0027keypairs\u0027, None)"},{"line_number":469,"context_line":"        if keypairs is not None:"},{"line_number":470,"context_line":"            expected_attrs.append(\u0027keypairs\u0027)"},{"line_number":471,"context_line":"            updates[\u0027extra\u0027][\u0027keypairs\u0027] \u003d jsonutils.dumps("},{"line_number":472,"context_line":"                keypairs.obj_to_primitive())"}],"source_content_type":"text/x-python","patch_set":9,"id":"dab17558_a1be45ab","line":469,"updated":"2016-05-11 17:53:58.000000000","message":"If this is busted without this change, it would have been good to have a unit test for it, but I won\u0027t continue pushing on this.","commit_id":"d8d171f59b360b2f608a8a81041f16d62a23dce1"}],"nova/tests/unit/compute/test_compute_api.py":[{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"cf93b3b4a8d67b21996baedc58347a61d9657bf7","unresolved":false,"context_lines":[{"line_number":212,"context_line":"            mock.patch.object(self.compute_api, \u0027_check_auto_disk_config\u0027),"},{"line_number":213,"context_line":"            mock.patch.object(self.compute_api,"},{"line_number":214,"context_line":"                              \u0027_validate_and_build_base_options\u0027,"},{"line_number":215,"context_line":"                              return_value\u003d({}, max_net_count, None))"},{"line_number":216,"context_line":"        ) as ("},{"line_number":217,"context_line":"            get_image,"},{"line_number":218,"context_line":"            check_auto_disk_config,"}],"source_content_type":"text/x-python","patch_set":5,"id":"dab17558_80577701","line":215,"updated":"2016-05-06 18:38:01.000000000","message":"Would it be good to add a test that handles the non-None condition?","commit_id":"42f94e813c4f5746e98205a6261311c5f884c56c"}]}