)]}'
{"nova/compute/api.py":[{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"1a2df8295c80eddfdfb969dfcbe4989cb8923f45","unresolved":false,"context_lines":[{"line_number":959,"context_line":"                # it\u0027s temporary for current patch"},{"line_number":960,"context_line":"                build_request.instance._changed_fields \u003d instance_changes"},{"line_number":961,"context_line":"                for i, bdm in enumerate(build_request.block_device_mappings):"},{"line_number":962,"context_line":"                    bdm._changed_fields \u003d bdm_changes[i]"},{"line_number":963,"context_line":"                build_requests.append(build_request)"},{"line_number":964,"context_line":"                # Create an instance_mapping.  The null cell_mapping indicates"},{"line_number":965,"context_line":"                # that the instance doesn\u0027t yet exist in a cell, and lookups"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a8bc5a2_170d8c37","line":962,"updated":"2016-09-27 19:41:40.000000000","message":"I think this logic belongs in the conductor method that will create the instance and block device mappings.","commit_id":"39f955b122455be71c560cfae60c68ea00c645f2"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"58a6e4b4d3bdc11c48daf05da934f18cb6ef505a","unresolved":false,"context_lines":[{"line_number":959,"context_line":"                # it\u0027s temporary for current patch"},{"line_number":960,"context_line":"                build_request.instance._changed_fields \u003d instance_changes"},{"line_number":961,"context_line":"                for i, bdm in enumerate(build_request.block_device_mappings):"},{"line_number":962,"context_line":"                    bdm._changed_fields \u003d bdm_changes[i]"},{"line_number":963,"context_line":"                build_requests.append(build_request)"},{"line_number":964,"context_line":"                # Create an instance_mapping.  The null cell_mapping indicates"},{"line_number":965,"context_line":"                # that the instance doesn\u0027t yet exist in a cell, and lookups"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a98d1f4_3c307801","line":962,"in_reply_to":"5a8bc5a2_170d8c37","updated":"2016-09-29 10:10:37.000000000","message":"Yep, this was bad. I added BuildRequest.get_new_instance method.","commit_id":"39f955b122455be71c560cfae60c68ea00c645f2"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"1a2df8295c80eddfdfb969dfcbe4989cb8923f45","unresolved":false,"context_lines":[{"line_number":1019,"context_line":"        # scenario. The only difference is the instance uuid, but that is used"},{"line_number":1020,"context_line":"        # to look it up from the database and not intended as input to the"},{"line_number":1021,"context_line":"        # scheduler."},{"line_number":1022,"context_line":"        # TODO(avolkov): make request_spec once"},{"line_number":1023,"context_line":"        request_spec \u003d request_specs[0]"},{"line_number":1024,"context_line":"        request_spec.num_instances \u003d len(build_requests)"},{"line_number":1025,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5a8bc5a2_2c5ddb48","line":1022,"updated":"2016-09-27 19:41:40.000000000","message":"Each instance needs it\u0027s own unique request spec in the database, even if only one is used now. But if an instance is later resized it\u0027s own request spec needs to be used and modified.","commit_id":"39f955b122455be71c560cfae60c68ea00c645f2"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"1a2df8295c80eddfdfb969dfcbe4989cb8923f45","unresolved":false,"context_lines":[{"line_number":1335,"context_line":"                        exception.InvalidVolume):"},{"line_number":1336,"context_line":"                    raise"},{"line_number":1337,"context_line":"                except Exception:"},{"line_number":1338,"context_line":"                    LOG.exception(_LE(\u0027Failed to check volume %s\u0027), volume_id)"},{"line_number":1339,"context_line":"                    raise exception.InvalidBDMVolume(id\u003dvolume_id)"},{"line_number":1340,"context_line":"            elif snapshot_id is not None:"},{"line_number":1341,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a8bc5a2_d729648b","line":1338,"updated":"2016-09-27 19:41:40.000000000","message":"This should be in a different patch.","commit_id":"39f955b122455be71c560cfae60c68ea00c645f2"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"58a6e4b4d3bdc11c48daf05da934f18cb6ef505a","unresolved":false,"context_lines":[{"line_number":1335,"context_line":"                        exception.InvalidVolume):"},{"line_number":1336,"context_line":"                    raise"},{"line_number":1337,"context_line":"                except Exception:"},{"line_number":1338,"context_line":"                    LOG.exception(_LE(\u0027Failed to check volume %s\u0027), volume_id)"},{"line_number":1339,"context_line":"                    raise exception.InvalidBDMVolume(id\u003dvolume_id)"},{"line_number":1340,"context_line":"            elif snapshot_id is not None:"},{"line_number":1341,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a98d1f4_af58ccfb","line":1338,"in_reply_to":"5a8bc5a2_d729648b","updated":"2016-09-29 10:10:37.000000000","message":"Removed from this patch. Don\u0027t sure these two lines worth a separate patch. I\u0027m thinking about hacking rule for such cases but \nit seems to me too complex at the moment.","commit_id":"39f955b122455be71c560cfae60c68ea00c645f2"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"1a2df8295c80eddfdfb969dfcbe4989cb8923f45","unresolved":false,"context_lines":[{"line_number":1345,"context_line":"                    raise"},{"line_number":1346,"context_line":"                except Exception:"},{"line_number":1347,"context_line":"                    LOG.exception(_LE(\u0027Failed to get snapshot %s\u0027),"},{"line_number":1348,"context_line":"                                  snapshot_id)"},{"line_number":1349,"context_line":"                    raise exception.InvalidBDMSnapshot(id\u003dsnapshot_id)"},{"line_number":1350,"context_line":"            elif (bdm.source_type \u003d\u003d \u0027blank\u0027 and"},{"line_number":1351,"context_line":"                    bdm.destination_type \u003d\u003d \u0027volume\u0027 and"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a8bc5a2_f71e605e","line":1348,"updated":"2016-09-27 19:41:40.000000000","message":"This should be in a different patch.","commit_id":"39f955b122455be71c560cfae60c68ea00c645f2"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5a4f5623c56b7934d3caa33599f64409514cccd5","unresolved":false,"context_lines":[{"line_number":1150,"context_line":""},{"line_number":1151,"context_line":"        instances \u003d []"},{"line_number":1152,"context_line":"        for build_request in build_requests:"},{"line_number":1153,"context_line":"            instance \u003d build_request.get_new_instance(context)"},{"line_number":1154,"context_line":"            instance.create()"},{"line_number":1155,"context_line":"            instances.append(instance)"},{"line_number":1156,"context_line":"            block_device_mapping \u003d ("},{"line_number":1157,"context_line":"                self._bdm_validate_set_size_and_instance("},{"line_number":1158,"context_line":"                    context, instance, instance_type, block_device_mapping))"},{"line_number":1159,"context_line":"            self._create_block_device_mapping(block_device_mapping)"},{"line_number":1160,"context_line":"            # send a state update notification for the initial create to"},{"line_number":1161,"context_line":"            # show it going from non-existent to BUILDING"},{"line_number":1162,"context_line":"            notifications.send_update_with_states(context, instance, None,"},{"line_number":1163,"context_line":"                    vm_states.BUILDING, None, None, service\u003d\"api\")"},{"line_number":1164,"context_line":""},{"line_number":1165,"context_line":"        for instance in instances:"},{"line_number":1166,"context_line":"            self._record_action_start(context, instance,"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba5da102_ef6a8280","line":1163,"range":{"start_line":1153,"start_character":0,"end_line":1163,"end_character":66},"updated":"2016-11-01 14:36:07.000000000","message":"Don\u0027t we need to be able to delete all the instances if any of this fails (like the code you\u0027re moving to here)?\n\nI would expect to throw this into a try/except block so we can destroy instances (and BDMs) that were created before the failure.","commit_id":"ad68b211152f04ea081071298600286a21d3ac24"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"f46ff6807edb45623c713f67ea626753dbc1a0f1","unresolved":false,"context_lines":[{"line_number":1008,"context_line":"        quotas.commit()"},{"line_number":1009,"context_line":"        # The request_specs are identical for each instance in a multiple boot"},{"line_number":1010,"context_line":"        # scenario. The only difference is the instance uuid, but that is used"},{"line_number":1011,"context_line":"        # to look it up from the database and not intended as input to the"},{"line_number":1012,"context_line":"        # scheduler."},{"line_number":1013,"context_line":"        request_spec \u003d request_specs[0]"},{"line_number":1014,"context_line":"        request_spec.num_instances \u003d len(build_requests)"},{"line_number":1015,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"ba5da102_aa1c716b","line":1012,"range":{"start_line":1011,"start_character":42,"end_line":1012,"end_character":20},"updated":"2016-11-04 14:07:46.000000000","message":"That\u0027s not true, the GroupAntiAffinityFilter makes use of the spec_obj.instance_uuid field.","commit_id":"12c06a7ec02872d88c848e9f00d4a771474d62e1"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"f46ff6807edb45623c713f67ea626753dbc1a0f1","unresolved":false,"context_lines":[{"line_number":1013,"context_line":"        request_spec \u003d request_specs[0]"},{"line_number":1014,"context_line":"        request_spec.num_instances \u003d len(build_requests)"},{"line_number":1015,"context_line":""},{"line_number":1016,"context_line":"        return build_requests, instance_mappings, request_specs[0]"},{"line_number":1017,"context_line":""},{"line_number":1018,"context_line":"    def _get_bdm_image_metadata(self, context, block_device_mapping,"},{"line_number":1019,"context_line":"                                legacy_bdm\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba5da102_6a67b9ea","line":1016,"range":{"start_line":1016,"start_character":50,"end_line":1016,"end_character":66},"updated":"2016-11-04 14:07:46.000000000","message":"I know this is borked, and Laski was in favor of deprecating the API parameter for supporting num_instances\u003e1 but here, I\u0027d prefer that you pass down the list of RequestSpec objects, because we could imagine some possibility in the future when Specs would be more diverging than just by the instance UUID (for example, if we want to stack all the instance creation calls).\n\nIdeally, I think the proper approach would be some iterable having a tuple of (build_request, instance_mapping, request_spec) for each instance.","commit_id":"12c06a7ec02872d88c848e9f00d4a771474d62e1"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"f46ff6807edb45623c713f67ea626753dbc1a0f1","unresolved":false,"context_lines":[{"line_number":1090,"context_line":"            try:"},{"line_number":1091,"context_line":"                req.destroy()"},{"line_number":1092,"context_line":"            except Exception:"},{"line_number":1093,"context_line":"                pass"},{"line_number":1094,"context_line":""},{"line_number":1095,"context_line":"    def _create_instance(self, context, instance_type,"},{"line_number":1096,"context_line":"               image_href, kernel_id, ramdisk_id,"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba5da102_4424abc5","line":1093,"updated":"2016-11-04 14:07:46.000000000","message":"you also need to destroy the RequestSpec that was attached to the faulty instance.\nFWIW, I wrote https://review.openstack.org/#/c/391060/ that targets to do that for a deletion request.","commit_id":"12c06a7ec02872d88c848e9f00d4a771474d62e1"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"f46ff6807edb45623c713f67ea626753dbc1a0f1","unresolved":false,"context_lines":[{"line_number":1160,"context_line":"        instance_group \u003d self._get_requested_instance_group(context,"},{"line_number":1161,"context_line":"                                   filter_properties)"},{"line_number":1162,"context_line":""},{"line_number":1163,"context_line":"        build_requests, instance_mappings, request_spec \u003d ("},{"line_number":1164,"context_line":"            self._provision_instances("},{"line_number":1165,"context_line":"                context, instance_type, min_count, max_count, base_options,"},{"line_number":1166,"context_line":"                boot_meta, security_groups, block_device_mapping,"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba5da102_8457a325","line":1163,"range":{"start_line":1163,"start_character":43,"end_line":1163,"end_character":55},"updated":"2016-11-04 14:07:46.000000000","message":"see my comment above, I\u0027d prefer to have a list of Specs.","commit_id":"12c06a7ec02872d88c848e9f00d4a771474d62e1"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"0792f42cf3d298eee4dac4762c04790628a9ea20","unresolved":false,"context_lines":[{"line_number":962,"context_line":"                inst_mapping.create()"},{"line_number":963,"context_line":""},{"line_number":964,"context_line":"                instances_to_build.append("},{"line_number":965,"context_line":"                    (req_spec, build_request, inst_mapping))"},{"line_number":966,"context_line":""},{"line_number":967,"context_line":"                if instance_group:"},{"line_number":968,"context_line":"                    if check_server_group_quota:"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a629dbe_ae2186a1","line":965,"updated":"2016-11-09 09:19:42.000000000","message":"I like this idea. Possibly, block_device_mapping can be added here to not duplicate _bdm_validate_set_size_and_instance call.","commit_id":"f8438e779432648c72458c46ad9e98c0af92788c"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"70fc93eb6f875f8703b5666fc23f88b0fe319120","unresolved":false,"context_lines":[{"line_number":1008,"context_line":""},{"line_number":1009,"context_line":"        # Commit the reservations"},{"line_number":1010,"context_line":"        quotas.commit()"},{"line_number":1011,"context_line":"        # The request_specs are identical for each instance in a multiple boot"},{"line_number":1012,"context_line":"        # scenario. The only difference is the instance uuid, but that is used"},{"line_number":1013,"context_line":"        # to look it up from the database and not intended as input to the"},{"line_number":1014,"context_line":"        # scheduler."},{"line_number":1015,"context_line":"        return instances_to_build"},{"line_number":1016,"context_line":""},{"line_number":1017,"context_line":"    def _get_bdm_image_metadata(self, context, block_device_mapping,"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a629dbe_ea78bcf4","line":1014,"range":{"start_line":1011,"start_character":0,"end_line":1014,"end_character":20},"updated":"2016-11-09 14:21:17.000000000","message":"you can remove that comment now that you\u0027re providing a tuple per instance.","commit_id":"f8438e779432648c72458c46ad9e98c0af92788c"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"0792f42cf3d298eee4dac4762c04790628a9ea20","unresolved":false,"context_lines":[{"line_number":1086,"context_line":"            except Exception:"},{"line_number":1087,"context_line":"                pass"},{"line_number":1088,"context_line":"        for req in build_requests:"},{"line_number":1089,"context_line":"            try:"},{"line_number":1090,"context_line":"                req.destroy()"},{"line_number":1091,"context_line":"            except Exception:"},{"line_number":1092,"context_line":"                pass"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a629dbe_539093ec","line":1089,"updated":"2016-11-09 09:19:42.000000000","message":"This cleanup is good but I don\u0027t sure we should destroy build request because build_request.destroy is a hard delete.\nIf we\u0027re moving instance creation to the conductor in the case of failure some record in DB has to stay to provide a proper answer to the user on the next list or show request.","commit_id":"f8438e779432648c72458c46ad9e98c0af92788c"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"70fc93eb6f875f8703b5666fc23f88b0fe319120","unresolved":false,"context_lines":[{"line_number":1086,"context_line":"            except Exception:"},{"line_number":1087,"context_line":"                pass"},{"line_number":1088,"context_line":"        for req in build_requests:"},{"line_number":1089,"context_line":"            try:"},{"line_number":1090,"context_line":"                req.destroy()"},{"line_number":1091,"context_line":"            except Exception:"},{"line_number":1092,"context_line":"                pass"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a629dbe_2ade3411","line":1089,"in_reply_to":"9a629dbe_539093ec","updated":"2016-11-09 14:21:17.000000000","message":"Andrey, you\u0027re missing the use of save_and_reraise_exception().\nAll of that is already present in the current code. Basically, if some problem happens when creating the instance, we\u0027re doing some handler mechanism to clean up the temporary bits *but* we reraise the original exception thanks to the context manager (default to True). That means that the user will still get the original instance error exception.","commit_id":"f8438e779432648c72458c46ad9e98c0af92788c"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"70fc93eb6f875f8703b5666fc23f88b0fe319120","unresolved":false,"context_lines":[{"line_number":1089,"context_line":"            try:"},{"line_number":1090,"context_line":"                req.destroy()"},{"line_number":1091,"context_line":"            except Exception:"},{"line_number":1092,"context_line":"                pass"},{"line_number":1093,"context_line":""},{"line_number":1094,"context_line":"    def _create_instance(self, context, instance_type,"},{"line_number":1095,"context_line":"               image_href, kernel_id, ramdisk_id,"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a629dbe_6ae90c80","line":1092,"updated":"2016-11-09 14:21:17.000000000","message":"you also need to delete the RequestSpec record as I mentioned in PS9.","commit_id":"f8438e779432648c72458c46ad9e98c0af92788c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"59b87dd96240b86a23b54eb7c297c305aea24469","unresolved":false,"context_lines":[{"line_number":1016,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":1017,"context_line":"                try:"},{"line_number":1018,"context_line":"                    for rs, br, im in instances_to_build:"},{"line_number":1019,"context_line":"                        try:"},{"line_number":1020,"context_line":"                            rs.destroy()"},{"line_number":1021,"context_line":"                        except exception.RequestSpecNotFound:"},{"line_number":1022,"context_line":"                            pass"},{"line_number":1023,"context_line":"                        try:"},{"line_number":1024,"context_line":"                            im.destroy()"},{"line_number":1025,"context_line":"                        except exception.InstanceMappingNotFound:"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_b5ba20bf","line":1022,"range":{"start_line":1019,"start_character":24,"end_line":1022,"end_character":32},"updated":"2016-11-22 21:57:09.000000000","message":"We never deleted the RequestSpec before, why do we care to now?","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a9bec4913767fc9145a1d474cd9b9f2a5d51bfaa","unresolved":false,"context_lines":[{"line_number":1016,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":1017,"context_line":"                try:"},{"line_number":1018,"context_line":"                    for rs, br, im in instances_to_build:"},{"line_number":1019,"context_line":"                        try:"},{"line_number":1020,"context_line":"                            rs.destroy()"},{"line_number":1021,"context_line":"                        except exception.RequestSpecNotFound:"},{"line_number":1022,"context_line":"                            pass"},{"line_number":1023,"context_line":"                        try:"},{"line_number":1024,"context_line":"                            im.destroy()"},{"line_number":1025,"context_line":"                        except exception.InstanceMappingNotFound:"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_9881d950","line":1022,"range":{"start_line":1019,"start_character":24,"end_line":1022,"end_character":32},"in_reply_to":"5a74a57a_b5ba20bf","updated":"2016-11-22 22:14:16.000000000","message":"Talked to Sylvain about this a couple weeks ago and he agreed it was a mistake to not delete it originally.","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"59b87dd96240b86a23b54eb7c297c305aea24469","unresolved":false,"context_lines":[{"line_number":1095,"context_line":""},{"line_number":1096,"context_line":"        return objects.InstanceGroup.get_by_uuid(context, group_hint)"},{"line_number":1097,"context_line":""},{"line_number":1098,"context_line":"    def _safe_destroy_instance_residue(self, instances, instances_to_build):"},{"line_number":1099,"context_line":"        for instance in instances:"},{"line_number":1100,"context_line":"            try:"},{"line_number":1101,"context_line":"                instance.destroy()"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_15f8347c","line":1098,"updated":"2016-11-22 21:57:09.000000000","message":"Docstrings on these parameters would be nice. The list of tuples for instances_to_build is going to be a bit wonky to remember a year from now.","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"53bfb2bb2c81e68f54ecc1c2d8e1db74d393ca6d","unresolved":false,"context_lines":[{"line_number":1099,"context_line":"        for instance in instances:"},{"line_number":1100,"context_line":"            try:"},{"line_number":1101,"context_line":"                instance.destroy()"},{"line_number":1102,"context_line":"            except Exception:"},{"line_number":1103,"context_line":"                pass"},{"line_number":1104,"context_line":"        for to_destroy in instances_to_build:"},{"line_number":1105,"context_line":"            for thing in to_destroy:"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_b52a207d","line":1102,"updated":"2016-11-22 21:39:58.000000000","message":"I wonder if we should debug log whatever happens here. I\u0027m wondering what else we would expect other than ObjectActionError.","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a9bec4913767fc9145a1d474cd9b9f2a5d51bfaa","unresolved":false,"context_lines":[{"line_number":1099,"context_line":"        for instance in instances:"},{"line_number":1100,"context_line":"            try:"},{"line_number":1101,"context_line":"                instance.destroy()"},{"line_number":1102,"context_line":"            except Exception:"},{"line_number":1103,"context_line":"                pass"},{"line_number":1104,"context_line":"        for to_destroy in instances_to_build:"},{"line_number":1105,"context_line":"            for thing in to_destroy:"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_3551b063","line":1102,"in_reply_to":"5a74a57a_b52a207d","updated":"2016-11-22 22:14:16.000000000","message":"A race to delete something I think, but sure.","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"59b87dd96240b86a23b54eb7c297c305aea24469","unresolved":false,"context_lines":[{"line_number":1099,"context_line":"        for instance in instances:"},{"line_number":1100,"context_line":"            try:"},{"line_number":1101,"context_line":"                instance.destroy()"},{"line_number":1102,"context_line":"            except Exception:"},{"line_number":1103,"context_line":"                pass"},{"line_number":1104,"context_line":"        for to_destroy in instances_to_build:"},{"line_number":1105,"context_line":"            for thing in to_destroy:"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_75d8881e","line":1102,"in_reply_to":"5a74a57a_b52a207d","updated":"2016-11-22 21:57:09.000000000","message":"Yeah this is a bit generic for my tastes for error handling, but it is called \u0027safe_destroy\u0027 for a reason I guess.","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"53bfb2bb2c81e68f54ecc1c2d8e1db74d393ca6d","unresolved":false,"context_lines":[{"line_number":1105,"context_line":"            for thing in to_destroy:"},{"line_number":1106,"context_line":"                try:"},{"line_number":1107,"context_line":"                    thing.destroy()"},{"line_number":1108,"context_line":"                except Exception:"},{"line_number":1109,"context_line":"                    pass"},{"line_number":1110,"context_line":""},{"line_number":1111,"context_line":"    def _create_instance(self, context, instance_type,"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_d51f9c59","line":1108,"updated":"2016-11-22 21:39:58.000000000","message":"And here","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"963d33681b96b874e95785aca7e344a6c31e9b40","unresolved":false,"context_lines":[{"line_number":1181,"context_line":"                context, instance_type, min_count, max_count, base_options,"},{"line_number":1182,"context_line":"                boot_meta, security_groups, block_device_mapping,"},{"line_number":1183,"context_line":"                shutdown_terminate, instance_group, check_server_group_quota,"},{"line_number":1184,"context_line":"                filter_properties, key_pair))"},{"line_number":1185,"context_line":""},{"line_number":1186,"context_line":"        instances \u003d []"},{"line_number":1187,"context_line":"        build_requests \u003d []"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_55c46c3f","line":1184,"updated":"2016-11-22 21:57:25.000000000","message":"you didn\u0027t need to change anything more than line 1179. could have just changed line 1179 to:\n\n instances_to_build \u003d self._provision_instances(context, instance_type,","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c3ce5cf0c4e9b530d3321a1c68ead8140d093202","unresolved":false,"context_lines":[{"line_number":1186,"context_line":"        instances \u003d []"},{"line_number":1187,"context_line":"        build_requests \u003d []"},{"line_number":1188,"context_line":"        instance_mappings \u003d []"},{"line_number":1189,"context_line":"        try:"},{"line_number":1190,"context_line":"            for rs, build_request, im in instances_to_build:"},{"line_number":1191,"context_line":"                build_requests.append(build_request)"},{"line_number":1192,"context_line":"                instance_mappings.append(im)"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_386525e1","line":1189,"updated":"2016-11-22 22:06:05.000000000","message":"The old code had these TODO comments which I think we should preserve for sanity:\n\n # TODO(alaski): Cast to conductor here which will call the\n # scheduler and defer instance creation until the scheduler\n # has picked a cell/host. Set the instance_mapping to the cell\n # that the instance is scheduled to.\n # NOTE(alaski): Instance and block device creation are going\n # to move to the conductor.\n\nUntil that actual move happens.","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"59b87dd96240b86a23b54eb7c297c305aea24469","unresolved":false,"context_lines":[{"line_number":1187,"context_line":"        build_requests \u003d []"},{"line_number":1188,"context_line":"        instance_mappings \u003d []"},{"line_number":1189,"context_line":"        try:"},{"line_number":1190,"context_line":"            for rs, build_request, im in instances_to_build:"},{"line_number":1191,"context_line":"                build_requests.append(build_request)"},{"line_number":1192,"context_line":"                instance_mappings.append(im)"},{"line_number":1193,"context_line":"                instance \u003d build_request.get_new_instance(context)"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_f539f842","line":1190,"range":{"start_line":1190,"start_character":16,"end_line":1190,"end_character":18},"updated":"2016-11-22 21:57:09.000000000","message":"nit: name this \u0027reqspec\u0027 so it\u0027s a bit more informative of what it is.","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"59b87dd96240b86a23b54eb7c297c305aea24469","unresolved":false,"context_lines":[{"line_number":1187,"context_line":"        build_requests \u003d []"},{"line_number":1188,"context_line":"        instance_mappings \u003d []"},{"line_number":1189,"context_line":"        try:"},{"line_number":1190,"context_line":"            for rs, build_request, im in instances_to_build:"},{"line_number":1191,"context_line":"                build_requests.append(build_request)"},{"line_number":1192,"context_line":"                instance_mappings.append(im)"},{"line_number":1193,"context_line":"                instance \u003d build_request.get_new_instance(context)"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_7546c8c0","line":1190,"range":{"start_line":1190,"start_character":35,"end_line":1190,"end_character":37},"updated":"2016-11-22 21:57:09.000000000","message":"s/im/instmap/ if it will fit on the same line?","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"59b87dd96240b86a23b54eb7c297c305aea24469","unresolved":false,"context_lines":[{"line_number":1188,"context_line":"        instance_mappings \u003d []"},{"line_number":1189,"context_line":"        try:"},{"line_number":1190,"context_line":"            for rs, build_request, im in instances_to_build:"},{"line_number":1191,"context_line":"                build_requests.append(build_request)"},{"line_number":1192,"context_line":"                instance_mappings.append(im)"},{"line_number":1193,"context_line":"                instance \u003d build_request.get_new_instance(context)"},{"line_number":1194,"context_line":"                instance.create()"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_35237007","line":1191,"range":{"start_line":1191,"start_character":16,"end_line":1191,"end_character":52},"updated":"2016-11-22 21:57:09.000000000","message":"Why do we need this? It\u0027s not used from what I can tell.","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"963d33681b96b874e95785aca7e344a6c31e9b40","unresolved":false,"context_lines":[{"line_number":1189,"context_line":"        try:"},{"line_number":1190,"context_line":"            for rs, build_request, im in instances_to_build:"},{"line_number":1191,"context_line":"                build_requests.append(build_request)"},{"line_number":1192,"context_line":"                instance_mappings.append(im)"},{"line_number":1193,"context_line":"                instance \u003d build_request.get_new_instance(context)"},{"line_number":1194,"context_line":"                instance.create()"},{"line_number":1195,"context_line":"                instances.append(instance)"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_b5086093","line":1192,"updated":"2016-11-22 21:57:25.000000000","message":"The instance_mappings variable is not used anywhere. I don\u0027t think you need it here.","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"59b87dd96240b86a23b54eb7c297c305aea24469","unresolved":false,"context_lines":[{"line_number":1189,"context_line":"        try:"},{"line_number":1190,"context_line":"            for rs, build_request, im in instances_to_build:"},{"line_number":1191,"context_line":"                build_requests.append(build_request)"},{"line_number":1192,"context_line":"                instance_mappings.append(im)"},{"line_number":1193,"context_line":"                instance \u003d build_request.get_new_instance(context)"},{"line_number":1194,"context_line":"                instance.create()"},{"line_number":1195,"context_line":"                instances.append(instance)"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_b57dc0e9","line":1192,"range":{"start_line":1192,"start_character":16,"end_line":1192,"end_character":44},"updated":"2016-11-22 21:57:09.000000000","message":"same here","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"53bfb2bb2c81e68f54ecc1c2d8e1db74d393ca6d","unresolved":false,"context_lines":[{"line_number":1194,"context_line":"                instance.create()"},{"line_number":1195,"context_line":"                instances.append(instance)"},{"line_number":1196,"context_line":"                block_device_mapping \u003d ("},{"line_number":1197,"context_line":"                    self._bdm_validate_set_size_and_instance("},{"line_number":1198,"context_line":"                        context, instance, instance_type,"},{"line_number":1199,"context_line":"                        block_device_mapping))"},{"line_number":1200,"context_line":"                self._create_block_device_mapping(block_device_mapping)"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_9581a450","line":1197,"updated":"2016-11-22 21:39:58.000000000","message":"I noticed this is being called twice now, once on L969 and once here. It seems like we could just get the block_device_mapping from the RequestSpec \u0027rs\u0027 since that\u0027s where we put it after validating it the first time. Or, could we remove the other one on L969? Is there any reason to do it twice?","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2bdb6eb3c79d7f0e7b96a04d9c8d3360c4f951b1","unresolved":false,"context_lines":[{"line_number":1194,"context_line":"                instance.create()"},{"line_number":1195,"context_line":"                instances.append(instance)"},{"line_number":1196,"context_line":"                block_device_mapping \u003d ("},{"line_number":1197,"context_line":"                    self._bdm_validate_set_size_and_instance("},{"line_number":1198,"context_line":"                        context, instance, instance_type,"},{"line_number":1199,"context_line":"                        block_device_mapping))"},{"line_number":1200,"context_line":"                self._create_block_device_mapping(block_device_mapping)"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_9b5f1375","line":1197,"in_reply_to":"5a74a57a_1813095e","updated":"2016-11-22 22:55:06.000000000","message":"So _bdm_validate_set_size_and_instance clones the BlockDeviceMappingList and stores the volume size and instance_uuid on each BDM in the list. Then that BDMList is stored on the BuildRequest. When BuildRequest.create() is called it serializes that list to store it in the DB and then hydrates it to set it back on the BuildRequest object, which loses the RequestContext making the BuildRequest.block_device_mapping an orphaned object. That\u0027s one issue.\n\nAlso see:\n\nhttps://review.openstack.org/#/c/367557/20/nova/conductor/manager.py@797\n\nWe\u0027re duplicating a bunch of this code and calling it twice now between compute API and conductor manager, which kind of sucks.\n\nI\u0027d prefer to just have _provision_instances return the cloned BlockDeviceMappingList and we pass that on to conductor (and use it here rather than call _bdm_validate_set_size_and_instance twice.","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a9bec4913767fc9145a1d474cd9b9f2a5d51bfaa","unresolved":false,"context_lines":[{"line_number":1194,"context_line":"                instance.create()"},{"line_number":1195,"context_line":"                instances.append(instance)"},{"line_number":1196,"context_line":"                block_device_mapping \u003d ("},{"line_number":1197,"context_line":"                    self._bdm_validate_set_size_and_instance("},{"line_number":1198,"context_line":"                        context, instance, instance_type,"},{"line_number":1199,"context_line":"                        block_device_mapping))"},{"line_number":1200,"context_line":"                self._create_block_device_mapping(block_device_mapping)"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_1813095e","line":1197,"in_reply_to":"5a74a57a_9581a450","updated":"2016-11-22 22:14:16.000000000","message":"I think because this goes away soon (I think). But yeah, can\u0027t think of any reason to do it again.","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"59b87dd96240b86a23b54eb7c297c305aea24469","unresolved":false,"context_lines":[{"line_number":1194,"context_line":"                instance.create()"},{"line_number":1195,"context_line":"                instances.append(instance)"},{"line_number":1196,"context_line":"                block_device_mapping \u003d ("},{"line_number":1197,"context_line":"                    self._bdm_validate_set_size_and_instance("},{"line_number":1198,"context_line":"                        context, instance, instance_type,"},{"line_number":1199,"context_line":"                        block_device_mapping))"},{"line_number":1200,"context_line":"                self._create_block_device_mapping(block_device_mapping)"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_55e54ca6","line":1197,"in_reply_to":"5a74a57a_9581a450","updated":"2016-11-22 21:57:09.000000000","message":"I thought the same thing, the block_device_mapping list is stored in the BuildRequest in _provision_instances. I think Mel means BuildRequest rather than RequestSpec.","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"2110383768b81ba4f96b74e07a1a7bf8011cc555","unresolved":false,"context_lines":[{"line_number":1113,"context_line":"            try:"},{"line_number":1114,"context_line":"                instance.destroy()"},{"line_number":1115,"context_line":"            except Exception as e:"},{"line_number":1116,"context_line":"                LOG.debug(\u0027Failed to destroy instance residue: %s\u0027 % e,"},{"line_number":1117,"context_line":"                          instance\u003dinstance)"},{"line_number":1118,"context_line":"        for to_destroy in instances_to_build:"},{"line_number":1119,"context_line":"            for thing in to_destroy:"}],"source_content_type":"text/x-python","patch_set":22,"id":"5a74a57a_c9cb811d","line":1116,"range":{"start_line":1116,"start_character":67,"end_line":1116,"end_character":68},"updated":"2016-11-23 00:37:36.000000000","message":"We got a new-ish hacking rule that enforces use of comma here.","commit_id":"df954d5f36b3bc70c4874b730498da3c2481b872"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"69cd6870eb9a76afc56146cf2b3c4f2810234860","unresolved":false,"context_lines":[{"line_number":1113,"context_line":"            try:"},{"line_number":1114,"context_line":"                instance.destroy()"},{"line_number":1115,"context_line":"            except Exception as e:"},{"line_number":1116,"context_line":"                LOG.debug(\u0027Failed to destroy instance residue: %s\u0027 % e,"},{"line_number":1117,"context_line":"                          instance\u003dinstance)"},{"line_number":1118,"context_line":"        for to_destroy in instances_to_build:"},{"line_number":1119,"context_line":"            for thing in to_destroy:"}],"source_content_type":"text/x-python","patch_set":22,"id":"5a74a57a_bf0c9b43","line":1116,"range":{"start_line":1116,"start_character":67,"end_line":1116,"end_character":68},"in_reply_to":"5a74a57a_c9cb811d","updated":"2016-11-23 14:47:29.000000000","message":"Yeah, I did it right below. Muscle memory is hard :(","commit_id":"df954d5f36b3bc70c4874b730498da3c2481b872"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b544baa236aab77334614f897961729cd82cd92a","unresolved":false,"context_lines":[{"line_number":1213,"context_line":"                instance.create()"},{"line_number":1214,"context_line":"                instances.append(instance)"},{"line_number":1215,"context_line":"                self._create_block_device_mapping("},{"line_number":1216,"context_line":"                    build_request.block_device_mappings)"},{"line_number":1217,"context_line":"                # send a state update notification for the initial create to"},{"line_number":1218,"context_line":"                # show it going from non-existent to BUILDING"},{"line_number":1219,"context_line":"                notifications.send_update_with_states(context, instance, None,"}],"source_content_type":"text/x-python","patch_set":22,"id":"5a74a57a_cf99c121","line":1216,"range":{"start_line":1216,"start_character":20,"end_line":1216,"end_character":55},"updated":"2016-11-23 02:00:09.000000000","message":"OK so now this is the result of the _bdm_validate_set_size_and_instance call which has the original clone with the context still on them, plus the size and instance uuid set. It\u0027s super tightly coupled, which makes me sad, but there aren\u0027t great alternatives.\n\nIf it were me, I\u0027d probably add a note here reminding us that this list is the one from _bdm_validate_set_size_and_instance.","commit_id":"df954d5f36b3bc70c4874b730498da3c2481b872"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7964546a2d948a1422c256d6764235b0693d8642","unresolved":false,"context_lines":[{"line_number":1107,"context_line":"        :param instances: List of Instance objects to destroy"},{"line_number":1108,"context_line":"        :param instances_to_build: List of tuples, output from"},{"line_number":1109,"context_line":"            _provision_instances, which is:"},{"line_number":1110,"context_line":"             build_request, request_spec, instance_mapping"},{"line_number":1111,"context_line":"        \"\"\""},{"line_number":1112,"context_line":"        for instance in instances:"},{"line_number":1113,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":23,"id":"5a74a57a_331b6474","line":1110,"range":{"start_line":1110,"start_character":13,"end_line":1110,"end_character":58},"updated":"2016-11-23 15:44:39.000000000","message":"nit: technically it\u0027s \"req_spec, build_request, inst_mapping\"","commit_id":"83f56ac4f498e1b24c1304b06eb2116da08d5a3a"}],"nova/objects/build_request.py":[{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"1a2df8295c80eddfdfb969dfcbe4989cb8923f45","unresolved":false,"context_lines":[{"line_number":102,"context_line":"        self.instance.created_at \u003d self.created_at"},{"line_number":103,"context_line":"        self.instance.updated_at \u003d self.updated_at"},{"line_number":104,"context_line":"        self.instance.tags \u003d objects.TagList(objects\u003d[])"},{"line_number":105,"context_line":"        self.instance._context \u003d self._context"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    def _load_block_device_mappings(self, db_bdms):"},{"line_number":108,"context_line":"        # \u0027db_bdms\u0027 is a serialized BlockDeviceMappingList object. If it\u0027s None"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a8bc5a2_b79938d3","line":105,"updated":"2016-09-27 19:41:40.000000000","message":"I specifically don\u0027t want this to happen. Rather than trying to save build_request.instance I think a new instance object should be created as a copy of build_request.instance. But build_request.instance.create() is an odd construct that I\u0027d rather avoid.","commit_id":"39f955b122455be71c560cfae60c68ea00c645f2"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"58a6e4b4d3bdc11c48daf05da934f18cb6ef505a","unresolved":false,"context_lines":[{"line_number":102,"context_line":"        self.instance.created_at \u003d self.created_at"},{"line_number":103,"context_line":"        self.instance.updated_at \u003d self.updated_at"},{"line_number":104,"context_line":"        self.instance.tags \u003d objects.TagList(objects\u003d[])"},{"line_number":105,"context_line":"        self.instance._context \u003d self._context"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    def _load_block_device_mappings(self, db_bdms):"},{"line_number":108,"context_line":"        # \u0027db_bdms\u0027 is a serialized BlockDeviceMappingList object. If it\u0027s None"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a98d1f4_0f1e200c","line":105,"in_reply_to":"5a8bc5a2_779ed064","updated":"2016-09-29 10:10:37.000000000","message":"Done","commit_id":"39f955b122455be71c560cfae60c68ea00c645f2"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"58a6e4b4d3bdc11c48daf05da934f18cb6ef505a","unresolved":false,"context_lines":[{"line_number":102,"context_line":"        self.instance.created_at \u003d self.created_at"},{"line_number":103,"context_line":"        self.instance.updated_at \u003d self.updated_at"},{"line_number":104,"context_line":"        self.instance.tags \u003d objects.TagList(objects\u003d[])"},{"line_number":105,"context_line":"        self.instance._context \u003d self._context"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    def _load_block_device_mappings(self, db_bdms):"},{"line_number":108,"context_line":"        # \u0027db_bdms\u0027 is a serialized BlockDeviceMappingList object. If it\u0027s None"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a98d1f4_cf2b582f","line":105,"in_reply_to":"5a8bc5a2_b79938d3","updated":"2016-09-29 10:10:37.000000000","message":"Done","commit_id":"39f955b122455be71c560cfae60c68ea00c645f2"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"ea4998b6089ae08104f835589ae554cb66f9666c","unresolved":false,"context_lines":[{"line_number":102,"context_line":"        self.instance.created_at \u003d self.created_at"},{"line_number":103,"context_line":"        self.instance.updated_at \u003d self.updated_at"},{"line_number":104,"context_line":"        self.instance.tags \u003d objects.TagList(objects\u003d[])"},{"line_number":105,"context_line":"        self.instance._context \u003d self._context"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    def _load_block_device_mappings(self, db_bdms):"},{"line_number":108,"context_line":"        # \u0027db_bdms\u0027 is a serialized BlockDeviceMappingList object. If it\u0027s None"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a8bc5a2_779ed064","line":105,"in_reply_to":"5a8bc5a2_b79938d3","updated":"2016-09-27 19:46:06.000000000","message":"I could see a helper method like get_new_instance which would return an instance object with context set and with a copy of all fields set like build_request.instance and all fields dirty. Then you could have something like:\n\n    instance \u003d build_request.get_new_instance()\n    instance.create()","commit_id":"39f955b122455be71c560cfae60c68ea00c645f2"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"1a2df8295c80eddfdfb969dfcbe4989cb8923f45","unresolved":false,"context_lines":[{"line_number":118,"context_line":""},{"line_number":119,"context_line":"        self.block_device_mappings \u003d ("},{"line_number":120,"context_line":"            objects.BlockDeviceMappingList.obj_from_primitive("},{"line_number":121,"context_line":"                jsonutils.loads(db_bdms), context\u003dself._context))"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"    @staticmethod"},{"line_number":124,"context_line":"    def _from_db_object(context, req, db_req):"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a8bc5a2_b780584b","line":121,"updated":"2016-09-27 19:41:40.000000000","message":"Same here. I don\u0027t want things like build_request.block_device_mappings.create(). Although in this case list objects don\u0027t have create/save methods anyways.","commit_id":"39f955b122455be71c560cfae60c68ea00c645f2"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"58a6e4b4d3bdc11c48daf05da934f18cb6ef505a","unresolved":false,"context_lines":[{"line_number":118,"context_line":""},{"line_number":119,"context_line":"        self.block_device_mappings \u003d ("},{"line_number":120,"context_line":"            objects.BlockDeviceMappingList.obj_from_primitive("},{"line_number":121,"context_line":"                jsonutils.loads(db_bdms), context\u003dself._context))"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"    @staticmethod"},{"line_number":124,"context_line":"    def _from_db_object(context, req, db_req):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a98d1f4_efa3b49c","line":121,"in_reply_to":"5a8bc5a2_b780584b","updated":"2016-09-29 10:10:37.000000000","message":"Done. In the case of a list it transfered context to its bdms.","commit_id":"39f955b122455be71c560cfae60c68ea00c645f2"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"f46ff6807edb45623c713f67ea626753dbc1a0f1","unresolved":false,"context_lines":[{"line_number":211,"context_line":"        db_req \u003d self._save_in_db(self._context, self.id, updates)"},{"line_number":212,"context_line":"        self._from_db_object(self._context, self, db_req)"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    def get_new_instance(self, context):"},{"line_number":215,"context_line":"        instance \u003d objects.Instance(context)"},{"line_number":216,"context_line":"        for field in self.instance.obj_fields:"},{"line_number":217,"context_line":"            if field \u003d\u003d \u0027tags\u0027:"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba5da102_6a1fb909","line":214,"range":{"start_line":214,"start_character":8,"end_line":214,"end_character":24},"updated":"2016-11-04 14:07:46.000000000","message":"Well, the naming could be confusing, we\u0027re not doing a lookup for getting the instance object, we\u0027re instanciating a new Instance object that will be persisted later.\n\nMaybe populate_instance(self) ? \nThat\u0027s a nit tho.","commit_id":"12c06a7ec02872d88c848e9f00d4a771474d62e1"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c3ce5cf0c4e9b530d3321a1c68ead8140d093202","unresolved":false,"context_lines":[{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    def get_new_instance(self, context):"},{"line_number":215,"context_line":"        instance \u003d objects.Instance(context)"},{"line_number":216,"context_line":"        for field in self.instance.obj_fields:"},{"line_number":217,"context_line":"            if field \u003d\u003d \u0027tags\u0027:"},{"line_number":218,"context_line":"                continue"},{"line_number":219,"context_line":"            if self.instance.obj_attr_is_set(field):"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_157754d5","line":216,"range":{"start_line":216,"start_character":21,"end_line":216,"end_character":34},"updated":"2016-11-22 22:06:05.000000000","message":"Should probably check that this is set before trying to use it, and fail with an ObjectActionError or something similar if it\u0027s not set.","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b02dbc668cb108884f9369e17119b5731649f704","unresolved":false,"context_lines":[{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    def get_new_instance(self, context):"},{"line_number":215,"context_line":"        instance \u003d objects.Instance(context)"},{"line_number":216,"context_line":"        for field in self.instance.obj_fields:"},{"line_number":217,"context_line":"            if field \u003d\u003d \u0027tags\u0027:"},{"line_number":218,"context_line":"                continue"},{"line_number":219,"context_line":"            if self.instance.obj_attr_is_set(field):"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_1e269116","line":216,"range":{"start_line":216,"start_character":21,"end_line":216,"end_character":34},"in_reply_to":"5a74a57a_157754d5","updated":"2016-11-22 23:23:51.000000000","message":"Why? This is a base property of VersionedObject and guaranteed to always be there unless we have something other than an objecthere (which we can\u0027t because our \"instance\" field will enforce it).","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"84090fde1291c560dcd6430daa17203eff604048","unresolved":false,"context_lines":[{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    def get_new_instance(self, context):"},{"line_number":215,"context_line":"        instance \u003d objects.Instance(context)"},{"line_number":216,"context_line":"        for field in self.instance.obj_fields:"},{"line_number":217,"context_line":"            if field \u003d\u003d \u0027tags\u0027:"},{"line_number":218,"context_line":"                continue"},{"line_number":219,"context_line":"            if self.instance.obj_attr_is_set(field):"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_4f6a111c","line":216,"range":{"start_line":216,"start_character":21,"end_line":216,"end_character":34},"in_reply_to":"5a74a57a_1e269116","updated":"2016-11-23 01:54:14.000000000","message":"I meant literally self.instance, i.e.:\n\nif not \u0027instance\u0027 in self:\n   fail","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"53bfb2bb2c81e68f54ecc1c2d8e1db74d393ca6d","unresolved":false,"context_lines":[{"line_number":214,"context_line":"    def get_new_instance(self, context):"},{"line_number":215,"context_line":"        instance \u003d objects.Instance(context)"},{"line_number":216,"context_line":"        for field in self.instance.obj_fields:"},{"line_number":217,"context_line":"            if field \u003d\u003d \u0027tags\u0027:"},{"line_number":218,"context_line":"                continue"},{"line_number":219,"context_line":"            if self.instance.obj_attr_is_set(field):"},{"line_number":220,"context_line":"                setattr(instance, field, getattr(self.instance, field))"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_750088ab","line":217,"updated":"2016-11-22 21:39:58.000000000","message":"Why skip tags?","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c3ce5cf0c4e9b530d3321a1c68ead8140d093202","unresolved":false,"context_lines":[{"line_number":214,"context_line":"    def get_new_instance(self, context):"},{"line_number":215,"context_line":"        instance \u003d objects.Instance(context)"},{"line_number":216,"context_line":"        for field in self.instance.obj_fields:"},{"line_number":217,"context_line":"            if field \u003d\u003d \u0027tags\u0027:"},{"line_number":218,"context_line":"                continue"},{"line_number":219,"context_line":"            if self.instance.obj_attr_is_set(field):"},{"line_number":220,"context_line":"                setattr(instance, field, getattr(self.instance, field))"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_352290ba","line":217,"in_reply_to":"5a74a57a_750088ab","updated":"2016-11-22 22:06:05.000000000","message":"Yeah...plus we don\u0027t even set tags on instances during the initial create so this wouldn\u0027t be set where this is used anyway.","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"53bfb2bb2c81e68f54ecc1c2d8e1db74d393ca6d","unresolved":false,"context_lines":[{"line_number":217,"context_line":"            if field \u003d\u003d \u0027tags\u0027:"},{"line_number":218,"context_line":"                continue"},{"line_number":219,"context_line":"            if self.instance.obj_attr_is_set(field):"},{"line_number":220,"context_line":"                setattr(instance, field, getattr(self.instance, field))"},{"line_number":221,"context_line":"        return instance"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_b50940e0","line":220,"updated":"2016-11-22 21:39:58.000000000","message":"Is it okay that these aren\u0027t cloning ObjectFields and refer to the same attr on self.instance? I guess I\u0027m not clear on the purpose of get_new_instance as it\u0027s not a obj_clone().","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"b5fbf97c27e50f9d3c583b2624dc58f6edc11fcd","unresolved":false,"context_lines":[{"line_number":217,"context_line":"            if field \u003d\u003d \u0027tags\u0027:"},{"line_number":218,"context_line":"                continue"},{"line_number":219,"context_line":"            if self.instance.obj_attr_is_set(field):"},{"line_number":220,"context_line":"                setattr(instance, field, getattr(self.instance, field))"},{"line_number":221,"context_line":"        return instance"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_c955331e","line":220,"in_reply_to":"5a74a57a_15267416","updated":"2016-11-23 09:55:09.000000000","message":"The main reason here is that we can\u0027t save instance without\nproperly set changed_fields attribute.\nWhen we\u0027re saving build_request with instance inside we\u0027re losing changed_fields for instance. Before instance creating\nwe have to do something to restore changed fields.\nI proposed to just create a new object and populate it with existing fields.","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a9bec4913767fc9145a1d474cd9b9f2a5d51bfaa","unresolved":false,"context_lines":[{"line_number":217,"context_line":"            if field \u003d\u003d \u0027tags\u0027:"},{"line_number":218,"context_line":"                continue"},{"line_number":219,"context_line":"            if self.instance.obj_attr_is_set(field):"},{"line_number":220,"context_line":"                setattr(instance, field, getattr(self.instance, field))"},{"line_number":221,"context_line":"        return instance"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_989ab9c4","line":220,"in_reply_to":"5a74a57a_15267416","updated":"2016-11-22 22:14:16.000000000","message":"Yeah, I\u0027m not sure why he did either of these, TBH. I\u0027ll see if I can replace this with clone.","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"963d33681b96b874e95785aca7e344a6c31e9b40","unresolved":false,"context_lines":[{"line_number":217,"context_line":"            if field \u003d\u003d \u0027tags\u0027:"},{"line_number":218,"context_line":"                continue"},{"line_number":219,"context_line":"            if self.instance.obj_attr_is_set(field):"},{"line_number":220,"context_line":"                setattr(instance, field, getattr(self.instance, field))"},{"line_number":221,"context_line":"        return instance"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_15267416","line":220,"in_reply_to":"5a74a57a_b50940e0","updated":"2016-11-22 21:57:25.000000000","message":"Yeah, I have similar questions as melwitt. Perhaps the only difference is the tags exclusion?","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b544baa236aab77334614f897961729cd82cd92a","unresolved":false,"context_lines":[{"line_number":218,"context_line":"        # so this is okay for the moment."},{"line_number":219,"context_line":"        instance \u003d objects.Instance(context)"},{"line_number":220,"context_line":"        for field in self.instance.obj_fields:"},{"line_number":221,"context_line":"            if field \u003d\u003d \u0027tags\u0027:"},{"line_number":222,"context_line":"                continue"},{"line_number":223,"context_line":"            if self.instance.obj_attr_is_set(field):"},{"line_number":224,"context_line":"                setattr(instance, field, getattr(self.instance, field))"},{"line_number":225,"context_line":"        return instance"}],"source_content_type":"text/x-python","patch_set":22,"id":"5a74a57a_4fadb130","line":222,"range":{"start_line":221,"start_character":12,"end_line":222,"end_character":24},"updated":"2016-11-23 02:00:09.000000000","message":"still not clear why this is excluded, it won\u0027t be set on server create anyway, at least not until this is implemented:\n\nhttps://blueprints.launchpad.net/nova/+spec/support-tag-instance-when-boot","commit_id":"df954d5f36b3bc70c4874b730498da3c2481b872"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"1f39f253ab391fb8e444efe04d6676c2eb4bf3ee","unresolved":false,"context_lines":[{"line_number":218,"context_line":"        # so this is okay for the moment."},{"line_number":219,"context_line":"        instance \u003d objects.Instance(context)"},{"line_number":220,"context_line":"        for field in self.instance.obj_fields:"},{"line_number":221,"context_line":"            if field \u003d\u003d \u0027tags\u0027:"},{"line_number":222,"context_line":"                continue"},{"line_number":223,"context_line":"            if self.instance.obj_attr_is_set(field):"},{"line_number":224,"context_line":"                setattr(instance, field, getattr(self.instance, field))"},{"line_number":225,"context_line":"        return instance"}],"source_content_type":"text/x-python","patch_set":22,"id":"5a74a57a_a93477de","line":222,"range":{"start_line":221,"start_character":12,"end_line":222,"end_character":24},"in_reply_to":"5a74a57a_4fadb130","updated":"2016-11-23 09:32:37.000000000","message":"Tags attribute breaks instance saving.\nThe issue is that newly created tags object like Tags(objects\u003d[]) can\u0027t be saved correctly due sqlalchemy doesn\u0027t consider it as a list object. It\u0027s definitely a workaround.","commit_id":"df954d5f36b3bc70c4874b730498da3c2481b872"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f36e76f3d16d4c8e9e219f2936ae6438a83ce021","unresolved":false,"context_lines":[{"line_number":218,"context_line":"        # so this is okay for the moment."},{"line_number":219,"context_line":"        instance \u003d objects.Instance(context)"},{"line_number":220,"context_line":"        for field in self.instance.obj_fields:"},{"line_number":221,"context_line":"            if field \u003d\u003d \u0027tags\u0027:"},{"line_number":222,"context_line":"                continue"},{"line_number":223,"context_line":"            if self.instance.obj_attr_is_set(field):"},{"line_number":224,"context_line":"                setattr(instance, field, getattr(self.instance, field))"},{"line_number":225,"context_line":"        return instance"}],"source_content_type":"text/x-python","patch_set":22,"id":"5a74a57a_e83c9731","line":222,"range":{"start_line":221,"start_character":12,"end_line":222,"end_character":24},"in_reply_to":"5a74a57a_a93477de","updated":"2016-11-23 15:38:50.000000000","message":"We should have a comment in the code then explaining that.","commit_id":"df954d5f36b3bc70c4874b730498da3c2481b872"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7617d09fbb2fe2eae29167a71e1e29268bd0316c","unresolved":false,"context_lines":[{"line_number":218,"context_line":"        # so this is okay for the moment."},{"line_number":219,"context_line":"        instance \u003d objects.Instance(context)"},{"line_number":220,"context_line":"        for field in self.instance.obj_fields:"},{"line_number":221,"context_line":"            if field \u003d\u003d \u0027tags\u0027:"},{"line_number":222,"context_line":"                continue"},{"line_number":223,"context_line":"            if self.instance.obj_attr_is_set(field):"},{"line_number":224,"context_line":"                setattr(instance, field, getattr(self.instance, field))"}],"source_content_type":"text/x-python","patch_set":23,"id":"5a74a57a_282dafa7","line":221,"range":{"start_line":221,"start_character":12,"end_line":221,"end_character":31},"updated":"2016-11-23 15:43:02.000000000","message":"nit: I\u0027d still like to see a comment here explaining why we need to skip tags. Andrey explained it in a previous patch set. I just don\u0027t want to have to remember that a year from now.","commit_id":"83f56ac4f498e1b24c1304b06eb2116da08d5a3a"}],"nova/objects/tag.py":[{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"1a2df8295c80eddfdfb969dfcbe4989cb8923f45","unresolved":false,"context_lines":[{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    # sqlalchemy doesn\u0027t recognize a newly created TagList as list"},{"line_number":61,"context_line":"    # see BuildRequest._load_instance"},{"line_number":62,"context_line":"    __emulates__ \u003d list"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    fields \u003d {"},{"line_number":65,"context_line":"        \u0027objects\u0027: fields.ListOfObjectsField(\u0027Tag\u0027),"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a8bc5a2_978c3c81","line":62,"updated":"2016-09-27 19:41:40.000000000","message":"Should be a separate patch.","commit_id":"39f955b122455be71c560cfae60c68ea00c645f2"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"58a6e4b4d3bdc11c48daf05da934f18cb6ef505a","unresolved":false,"context_lines":[{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    # sqlalchemy doesn\u0027t recognize a newly created TagList as list"},{"line_number":61,"context_line":"    # see BuildRequest._load_instance"},{"line_number":62,"context_line":"    __emulates__ \u003d list"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    fields \u003d {"},{"line_number":65,"context_line":"        \u0027objects\u0027: fields.ListOfObjectsField(\u0027Tag\u0027),"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a98d1f4_2f94bce0","line":62,"in_reply_to":"5a8bc5a2_978c3c81","updated":"2016-09-29 10:10:37.000000000","message":"It was required because instance.tags was TagList([]).\nFor such case instance couldn\u0027t be created because sqlalchemy did not recognize it as list object and failed.\nNow I just clear instance.tags in BuildRequest.get_new_instance\nand all goes well.","commit_id":"39f955b122455be71c560cfae60c68ea00c645f2"}],"nova/tests/unit/api/openstack/compute/test_block_device_mapping.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c3ce5cf0c4e9b530d3321a1c68ead8140d093202","unresolved":false,"context_lines":[{"line_number":135,"context_line":""},{"line_number":136,"context_line":"        self.stubs.Set(compute_api.API, \u0027create\u0027, create)"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"        self.mox.StubOutWithMock(compute_api.API, \u0027_validate_bdm\u0027)"},{"line_number":139,"context_line":"        self.mox.StubOutWithMock(compute_api.API, \u0027_get_bdm_image_metadata\u0027)"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"        compute_api.API._validate_bdm("},{"line_number":142,"context_line":"            mox.IgnoreArg(), mox.IgnoreArg(), mox.IgnoreArg(),"},{"line_number":143,"context_line":"            mox.IgnoreArg()).AndReturn(True)"},{"line_number":144,"context_line":"        compute_api.API._get_bdm_image_metadata("},{"line_number":145,"context_line":"            mox.IgnoreArg(), mox.IgnoreArg(), False).AndReturn({})"},{"line_number":146,"context_line":"        self.mox.ReplayAll()"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"        params \u003d {block_device_mapping.ATTRIBUTE_NAME: self.bdm}"},{"line_number":149,"context_line":"        self._test_create(params, no_image\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_f8a9ede0","side":"PARENT","line":146,"range":{"start_line":138,"start_character":8,"end_line":146,"end_character":28},"updated":"2016-11-22 22:06:05.000000000","message":"This seems like unnecessary cleanup when you didn\u0027t even need to touch this test case.","commit_id":"e9a20fdfb90596faa795930a48b4f57e2826cc5b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d21502e2291f451d49b18f422d936b35238afbad","unresolved":false,"context_lines":[{"line_number":135,"context_line":""},{"line_number":136,"context_line":"        self.stubs.Set(compute_api.API, \u0027create\u0027, create)"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"        self.mox.StubOutWithMock(compute_api.API, \u0027_validate_bdm\u0027)"},{"line_number":139,"context_line":"        self.mox.StubOutWithMock(compute_api.API, \u0027_get_bdm_image_metadata\u0027)"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"        compute_api.API._validate_bdm("},{"line_number":142,"context_line":"            mox.IgnoreArg(), mox.IgnoreArg(), mox.IgnoreArg(),"},{"line_number":143,"context_line":"            mox.IgnoreArg()).AndReturn(True)"},{"line_number":144,"context_line":"        compute_api.API._get_bdm_image_metadata("},{"line_number":145,"context_line":"            mox.IgnoreArg(), mox.IgnoreArg(), False).AndReturn({})"},{"line_number":146,"context_line":"        self.mox.ReplayAll()"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"        params \u003d {block_device_mapping.ATTRIBUTE_NAME: self.bdm}"},{"line_number":149,"context_line":"        self._test_create(params, no_image\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_de43396b","side":"PARENT","line":146,"range":{"start_line":138,"start_character":8,"end_line":146,"end_character":28},"in_reply_to":"5a74a57a_f8a9ede0","updated":"2016-11-22 23:34:04.000000000","message":"This is because it was all getting called twice as part of the create call I think.","commit_id":"e9a20fdfb90596faa795930a48b4f57e2826cc5b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c3ce5cf0c4e9b530d3321a1c68ead8140d093202","unresolved":false,"context_lines":[{"line_number":124,"context_line":""},{"line_number":125,"context_line":"    @mock.patch.object(compute_api.API, \u0027_get_bdm_image_metadata\u0027,"},{"line_number":126,"context_line":"                       return_value\u003d{})"},{"line_number":127,"context_line":"    @mock.patch.object(compute_api.API, \u0027_validate_bdm\u0027)"},{"line_number":128,"context_line":"    def test_create_instance_with_bdms_and_no_image("},{"line_number":129,"context_line":"            self, _validate_bdm, _get_bdm_image_metadata):"},{"line_number":130,"context_line":"        old_create \u003d compute_api.API.create"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_788ddd7a","line":127,"updated":"2016-11-22 22:06:05.000000000","message":"This should have return_value\u003dTrue if it\u0027s going to be the same as before.","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"}],"nova/tests/unit/api/openstack/compute/test_block_device_mapping_v1.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4b844d26ec35c7b7c76442c4eb5d6854e59d2427","unresolved":false,"context_lines":[{"line_number":109,"context_line":"        self.stubs.Set(compute_api.API, \u0027_validate_bdm\u0027, _validate_bdm)"},{"line_number":110,"context_line":"        self._test_create(params)"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    @mock.patch.object(compute_api.API, \u0027_get_bdm_image_metadata\u0027)"},{"line_number":113,"context_line":"    @mock.patch.object(compute_api.API, \u0027_validate_bdm\u0027)"},{"line_number":114,"context_line":"    def test_create_instance_with_volumes_enabled_and_bdms_no_image("},{"line_number":115,"context_line":"            self, _validate_bdm, _get_bdm_image_metadata):"},{"line_number":116,"context_line":"        \"\"\"Test that the create works if there is no image supplied but"},{"line_number":117,"context_line":"        os-volumes extension is enabled and bdms are supplied"},{"line_number":118,"context_line":"        \"\"\""},{"line_number":119,"context_line":"        volume \u003d {"},{"line_number":120,"context_line":"            \u0027id\u0027: uuids.volume_id,"},{"line_number":121,"context_line":"            \u0027status\u0027: \u0027active\u0027,"},{"line_number":122,"context_line":"            \u0027volume_image_metadata\u0027:"},{"line_number":123,"context_line":"                {\u0027test_key\u0027: \u0027test_value\u0027}"},{"line_number":124,"context_line":"        }"},{"line_number":125,"context_line":"        _get_bdm_image_metadata.return_value \u003d volume"},{"line_number":126,"context_line":"        params \u003d {\u0027block_device_mapping\u0027: self.bdm}"},{"line_number":127,"context_line":"        old_create \u003d compute_api.API.create"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        def create(*args, **kwargs):"},{"line_number":130,"context_line":"            self.assertEqual(kwargs[\u0027block_device_mapping\u0027], self.bdm)"},{"line_number":131,"context_line":"            self.assertNotIn(\u0027imageRef\u0027, kwargs)"},{"line_number":132,"context_line":"            return old_create(*args, **kwargs)"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"        self.stubs.Set(compute_api.API, \u0027create\u0027, create)"},{"line_number":135,"context_line":"        self._test_create(params, no_image\u003dTrue)"},{"line_number":136,"context_line":"        self.assertTrue(_validate_bdm.called)"},{"line_number":137,"context_line":"        _get_bdm_image_metadata.assert_called_with(mock.ANY, self.bdm, True)"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"    @mock.patch.object(compute_api.API, \u0027_validate_bdm\u0027)"},{"line_number":140,"context_line":"    @mock.patch.object(compute_api.API, \u0027_get_bdm_image_metadata\u0027)"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_585581c4","line":137,"range":{"start_line":112,"start_character":4,"end_line":137,"end_character":76},"updated":"2016-11-22 22:10:59.000000000","message":"Why do we need to change any of this in this patch? It all seems like unrelated cleanup which just clutters up this patch.","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d21502e2291f451d49b18f422d936b35238afbad","unresolved":false,"context_lines":[{"line_number":109,"context_line":"        self.stubs.Set(compute_api.API, \u0027_validate_bdm\u0027, _validate_bdm)"},{"line_number":110,"context_line":"        self._test_create(params)"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    @mock.patch.object(compute_api.API, \u0027_get_bdm_image_metadata\u0027)"},{"line_number":113,"context_line":"    @mock.patch.object(compute_api.API, \u0027_validate_bdm\u0027)"},{"line_number":114,"context_line":"    def test_create_instance_with_volumes_enabled_and_bdms_no_image("},{"line_number":115,"context_line":"            self, _validate_bdm, _get_bdm_image_metadata):"},{"line_number":116,"context_line":"        \"\"\"Test that the create works if there is no image supplied but"},{"line_number":117,"context_line":"        os-volumes extension is enabled and bdms are supplied"},{"line_number":118,"context_line":"        \"\"\""},{"line_number":119,"context_line":"        volume \u003d {"},{"line_number":120,"context_line":"            \u0027id\u0027: uuids.volume_id,"},{"line_number":121,"context_line":"            \u0027status\u0027: \u0027active\u0027,"},{"line_number":122,"context_line":"            \u0027volume_image_metadata\u0027:"},{"line_number":123,"context_line":"                {\u0027test_key\u0027: \u0027test_value\u0027}"},{"line_number":124,"context_line":"        }"},{"line_number":125,"context_line":"        _get_bdm_image_metadata.return_value \u003d volume"},{"line_number":126,"context_line":"        params \u003d {\u0027block_device_mapping\u0027: self.bdm}"},{"line_number":127,"context_line":"        old_create \u003d compute_api.API.create"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        def create(*args, **kwargs):"},{"line_number":130,"context_line":"            self.assertEqual(kwargs[\u0027block_device_mapping\u0027], self.bdm)"},{"line_number":131,"context_line":"            self.assertNotIn(\u0027imageRef\u0027, kwargs)"},{"line_number":132,"context_line":"            return old_create(*args, **kwargs)"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"        self.stubs.Set(compute_api.API, \u0027create\u0027, create)"},{"line_number":135,"context_line":"        self._test_create(params, no_image\u003dTrue)"},{"line_number":136,"context_line":"        self.assertTrue(_validate_bdm.called)"},{"line_number":137,"context_line":"        _get_bdm_image_metadata.assert_called_with(mock.ANY, self.bdm, True)"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"    @mock.patch.object(compute_api.API, \u0027_validate_bdm\u0027)"},{"line_number":140,"context_line":"    @mock.patch.object(compute_api.API, \u0027_get_bdm_image_metadata\u0027)"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_7e55c51b","line":137,"range":{"start_line":112,"start_character":4,"end_line":137,"end_character":76},"in_reply_to":"5a74a57a_585581c4","updated":"2016-11-22 23:34:04.000000000","message":"Same as the last file, I think.","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"}],"nova/tests/unit/compute/test_compute.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"963d33681b96b874e95785aca7e344a6c31e9b40","unresolved":false,"context_lines":[{"line_number":7748,"context_line":"                id\u003d0, cpuset\u003dset([1, 2]), memory\u003d512),"},{"line_number":7749,"context_line":"                   objects.InstanceNUMACell("},{"line_number":7750,"context_line":"                id\u003d1, cpuset\u003dset([3, 4]), memory\u003d512)])"},{"line_number":7751,"context_line":"        numa_topology.obj_reset_changes()"},{"line_number":7752,"context_line":"        numa_constraints_mock.return_value \u003d numa_topology"},{"line_number":7753,"context_line":""},{"line_number":7754,"context_line":"        instances, resv_id \u003d self.compute_api.create(self.context, inst_type,"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_9578c4fa","line":7751,"updated":"2016-11-22 21:57:25.000000000","message":"hmm, this change is a bit suspicious... why the need for it?","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d21502e2291f451d49b18f422d936b35238afbad","unresolved":false,"context_lines":[{"line_number":7748,"context_line":"                id\u003d0, cpuset\u003dset([1, 2]), memory\u003d512),"},{"line_number":7749,"context_line":"                   objects.InstanceNUMACell("},{"line_number":7750,"context_line":"                id\u003d1, cpuset\u003dset([3, 4]), memory\u003d512)])"},{"line_number":7751,"context_line":"        numa_topology.obj_reset_changes()"},{"line_number":7752,"context_line":"        numa_constraints_mock.return_value \u003d numa_topology"},{"line_number":7753,"context_line":""},{"line_number":7754,"context_line":"        instances, resv_id \u003d self.compute_api.create(self.context, inst_type,"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_fe3f15d4","line":7751,"in_reply_to":"5a74a57a_9578c4fa","updated":"2016-11-22 23:34:04.000000000","message":"Because we\u0027re now putting these into an object that gets create()d, and does a recursive obj_reset_changes() call.","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"}],"nova/tests/unit/compute/test_compute_api.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"f46ff6807edb45623c713f67ea626753dbc1a0f1","unresolved":false,"context_lines":[{"line_number":3723,"context_line":"                              shutdown_terminate, instance_group,"},{"line_number":3724,"context_line":"                              check_server_group_quota, filter_properties,"},{"line_number":3725,"context_line":"                              None)"},{"line_number":3726,"context_line":"            # First instance, build_req, mapping is created and destroyed"},{"line_number":3727,"context_line":"            self.assertTrue(build_req_mocks[0].create.called)"},{"line_number":3728,"context_line":"            self.assertTrue(build_req_mocks[0].destroy.called)"},{"line_number":3729,"context_line":"            self.assertTrue(inst_map_mocks[0].create.called)"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba5da102_aac71163","line":3726,"range":{"start_line":3726,"start_character":20,"end_line":3726,"end_character":29},"updated":"2016-11-04 14:07:46.000000000","message":"you should amend that comment","commit_id":"12c06a7ec02872d88c848e9f00d4a771474d62e1"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"963d33681b96b874e95785aca7e344a6c31e9b40","unresolved":false,"context_lines":[{"line_number":3626,"context_line":"            filter_properties \u003d {\u0027scheduler_hints\u0027: None,"},{"line_number":3627,"context_line":"                    \u0027instance_type\u0027: flavor}"},{"line_number":3628,"context_line":""},{"line_number":3629,"context_line":"            instances_to_build \u003d ("},{"line_number":3630,"context_line":"                self.compute_api._provision_instances(ctxt, flavor,"},{"line_number":3631,"context_line":"                    min_count, max_count, base_options, boot_meta,"},{"line_number":3632,"context_line":"                    security_groups, block_device_mapping, shutdown_terminate,"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_d5d07ccc","line":3629,"updated":"2016-11-22 21:57:25.000000000","message":"same comment as before, there is no need to change anything other than line 3629 here :)","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a9bec4913767fc9145a1d474cd9b9f2a5d51bfaa","unresolved":false,"context_lines":[{"line_number":3626,"context_line":"            filter_properties \u003d {\u0027scheduler_hints\u0027: None,"},{"line_number":3627,"context_line":"                    \u0027instance_type\u0027: flavor}"},{"line_number":3628,"context_line":""},{"line_number":3629,"context_line":"            instances_to_build \u003d ("},{"line_number":3630,"context_line":"                self.compute_api._provision_instances(ctxt, flavor,"},{"line_number":3631,"context_line":"                    min_count, max_count, base_options, boot_meta,"},{"line_number":3632,"context_line":"                    security_groups, block_device_mapping, shutdown_terminate,"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_b86215ac","line":3629,"in_reply_to":"5a74a57a_d5d07ccc","updated":"2016-11-22 22:14:16.000000000","message":"It clearly won\u0027t fit on the line without changing the args. If I go for minimum damage, then I have that line, plus ctxt and flavor on their own line next, followed by longer lines. This seemed like the smallest amount of change to me without just making ugly.","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"}],"nova/tests/unit/objects/test_build_request.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"963d33681b96b874e95785aca7e344a6c31e9b40","unresolved":false,"context_lines":[{"line_number":136,"context_line":"    def test_get_new_instance_show_changed_fields(self):"},{"line_number":137,"context_line":"        fake_req \u003d fake_build_request.fake_db_req()"},{"line_number":138,"context_line":"        fields \u003d jsonutils.loads(fake_req[\u0027instance\u0027])[\u0027nova_object.data\u0027]"},{"line_number":139,"context_line":"        self.assertTrue(fields)"},{"line_number":140,"context_line":"        build_request \u003d objects.BuildRequest._from_db_object("},{"line_number":141,"context_line":"            self.context, objects.BuildRequest(), fake_req)"},{"line_number":142,"context_line":"        self.assertFalse(build_request.instance.obj_what_changed())"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_d5e79caa","line":139,"updated":"2016-11-22 21:57:25.000000000","message":"assertTrue?","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"963d33681b96b874e95785aca7e344a6c31e9b40","unresolved":false,"context_lines":[{"line_number":139,"context_line":"        self.assertTrue(fields)"},{"line_number":140,"context_line":"        build_request \u003d objects.BuildRequest._from_db_object("},{"line_number":141,"context_line":"            self.context, objects.BuildRequest(), fake_req)"},{"line_number":142,"context_line":"        self.assertFalse(build_request.instance.obj_what_changed())"},{"line_number":143,"context_line":"        instance \u003d build_request.get_new_instance(self.context)"},{"line_number":144,"context_line":"        for field in fields:"},{"line_number":145,"context_line":"            self.assertIn(field, instance.obj_what_changed())"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_55fc6c3e","line":142,"updated":"2016-11-22 21:57:25.000000000","message":"assertFalse? Perhaps better to do:\n\n self.assertEqual(0, len(build_request.instance.obj_what_changed()))","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"963d33681b96b874e95785aca7e344a6c31e9b40","unresolved":false,"context_lines":[{"line_number":144,"context_line":"        for field in fields:"},{"line_number":145,"context_line":"            self.assertIn(field, instance.obj_what_changed())"},{"line_number":146,"context_line":"            self.assertEqual(getattr(build_request.instance, field),"},{"line_number":147,"context_line":"                             getattr(instance, field))"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"class TestBuildRequestObject(test_objects._LocalTest,"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_b5c060e2","line":147,"updated":"2016-11-22 21:57:25.000000000","message":"Might be better to just do:\n\n self.assertObjectEqual(build_request.instance, instance)","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"4265e7699ed01789be25b195824527f79cb0728f","unresolved":false,"context_lines":[{"line_number":144,"context_line":"        for field in fields:"},{"line_number":145,"context_line":"            self.assertIn(field, instance.obj_what_changed())"},{"line_number":146,"context_line":"            self.assertEqual(getattr(build_request.instance, field),"},{"line_number":147,"context_line":"                             getattr(instance, field))"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"class TestBuildRequestObject(test_objects._LocalTest,"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_edad8932","line":147,"in_reply_to":"5a74a57a_a9d2c532","updated":"2016-11-23 15:23:51.000000000","message":"No, I was thinking of https://github.com/openstack/nova/blob/master/nova/tests/unit/db/test_db_api.py#L1600 mistakenly. :(\n\nSorry about that. Not sure what I was going for there.","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"20ff06e9c04ace816e7a0c3443db0fa14ff49224","unresolved":false,"context_lines":[{"line_number":144,"context_line":"        for field in fields:"},{"line_number":145,"context_line":"            self.assertIn(field, instance.obj_what_changed())"},{"line_number":146,"context_line":"            self.assertEqual(getattr(build_request.instance, field),"},{"line_number":147,"context_line":"                             getattr(instance, field))"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"class TestBuildRequestObject(test_objects._LocalTest,"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a74a57a_a9d2c532","line":147,"in_reply_to":"5a74a57a_b5c060e2","updated":"2016-11-23 00:01:15.000000000","message":"Do you mean:\n\n assertTrue(objects_base.obj_equal_prims( .. ))\n\n?\n\nIf so, that\u0027s not what he\u0027s going for here, because the dirtiness of the fields is important and different from the base object\u0027s instance.","commit_id":"b14a6d9ae3d739044e17bba2727af5b7ffb478ab"}]}