)]}'
{"nova/compute/api.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"426956ecef075e74218e6b55e95e53a1c7f46c3f","unresolved":false,"context_lines":[{"line_number":966,"context_line":"                # an instance table. At that point it will never be used again"},{"line_number":967,"context_line":"                # and should be deleted. As instance creation is pushed back,"},{"line_number":968,"context_line":"                # as noted above, this will move with it."},{"line_number":969,"context_line":"                build_request.destroy()"},{"line_number":970,"context_line":""},{"line_number":971,"context_line":"                if instance_group:"},{"line_number":972,"context_line":"                    if check_server_group_quota:"}],"source_content_type":"text/x-python","patch_set":11,"id":"7af24918_e422a700","line":969,"updated":"2016-03-02 16:26:45.000000000","message":"Hmm, do we need to sync any changes written into the build_request, like updates to locked_by? after the instance has been created in the cell DB?\n\nClearly that doesn\u0027t matter yet, just thinking longer term.","commit_id":"16755889c6f35adf282ba459bdddbdd54cddce3e"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d9a56f2ce9531434dc3e6e09e657881354972353","unresolved":false,"context_lines":[{"line_number":966,"context_line":"                # an instance table. At that point it will never be used again"},{"line_number":967,"context_line":"                # and should be deleted. As instance creation is pushed back,"},{"line_number":968,"context_line":"                # as noted above, this will move with it."},{"line_number":969,"context_line":"                build_request.destroy()"},{"line_number":970,"context_line":""},{"line_number":971,"context_line":"                if instance_group:"},{"line_number":972,"context_line":"                    if check_server_group_quota:"}],"source_content_type":"text/x-python","patch_set":11,"id":"7af24918_77a2cbaf","line":969,"in_reply_to":"7af24918_e422a700","updated":"2016-03-02 16:33:21.000000000","message":"Thinking on this more, lets be sure to include a TODO in here, for when the API actually updates the build_request and not the instance object?","commit_id":"16755889c6f35adf282ba459bdddbdd54cddce3e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b61c02c768cf99cd3302b9b4dd6c11312df0a27b","unresolved":false,"context_lines":[{"line_number":921,"context_line":"                        base_options[\u0027pci_requests\u0027], filter_properties,"},{"line_number":922,"context_line":"                        instance_group, base_options[\u0027availability_zone\u0027])"},{"line_number":923,"context_line":"                req_spec.create()"},{"line_number":924,"context_line":"                # Store the BuildRequest that will help populate an instance"},{"line_number":925,"context_line":"                # object for a list/show request"},{"line_number":926,"context_line":"                info_cache \u003d objects.InstanceInfoCache()"},{"line_number":927,"context_line":"                info_cache.instance_uuid \u003d instance_uuid"}],"source_content_type":"text/x-python","patch_set":12,"id":"7af24918_43009f01","line":924,"updated":"2016-03-02 20:33:28.000000000","message":"nit: I\u0027d prefer to move this big block of code that\u0027s creating the build request into a separate private method, just because this method is getting large.  The private method gets the request details it needs, creates the build request in the database and returns it so we can destroy it later after the instance is created.","commit_id":"e7020d130b1401cee84aaf80287dcd4be9feea89"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b61c02c768cf99cd3302b9b4dd6c11312df0a27b","unresolved":false,"context_lines":[{"line_number":926,"context_line":"                info_cache \u003d objects.InstanceInfoCache()"},{"line_number":927,"context_line":"                info_cache.instance_uuid \u003d instance_uuid"},{"line_number":928,"context_line":"                info_cache.network_info \u003d network_model.NetworkInfo()"},{"line_number":929,"context_line":"                sec_groups \u003d self.security_group_api.populate_security_groups("},{"line_number":930,"context_line":"                        security_groups)"},{"line_number":931,"context_line":"                # NOTE: base_options[\u0027config_drive\u0027] is either True or \u0027\u0027 due"},{"line_number":932,"context_line":"                # to how it\u0027s represented in the instances table in the db."},{"line_number":933,"context_line":"                # BuildRequest needs a boolean."}],"source_content_type":"text/x-python","patch_set":12,"id":"7af24918_43837f56","line":930,"range":{"start_line":929,"start_character":16,"end_line":930,"end_character":40},"updated":"2016-03-02 20:33:28.000000000","message":"I\u0027d like to pass this down through create_db_entry_for_new_instance so it can be used in _populate_instance_for_create so we aren\u0027t making redundant calls to the security group API for each instance (twice per instance now b/c of the build request stuff). I\u0027d like to avoid round trips to neutron if we can help it.\n\nAlso, now that I look at this, we should totally be populating the security groups outside of the for loop since there is nothing instance specific about that call from that I can tell.\n\nAlthough now that I look, we don\u0027t actually call neutron for that method:\n\nhttps://github.com/openstack/nova/blob/master/nova/network/security_group/neutron_driver.py#L548-L551\n\nBut still, it seems like a thing we should do outside the for loop and pass it on down through to _populate_instance_for_create.\n\nIt\u0027s also pretty simple for nova-network:\n\nhttps://github.com/openstack/nova/blob/master/nova/compute/api.py#L4332-L4336","commit_id":"e7020d130b1401cee84aaf80287dcd4be9feea89"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b61c02c768cf99cd3302b9b4dd6c11312df0a27b","unresolved":false,"context_lines":[{"line_number":931,"context_line":"                # NOTE: base_options[\u0027config_drive\u0027] is either True or \u0027\u0027 due"},{"line_number":932,"context_line":"                # to how it\u0027s represented in the instances table in the db."},{"line_number":933,"context_line":"                # BuildRequest needs a boolean."},{"line_number":934,"context_line":"                bool_config_drive \u003d (True if base_options[\u0027config_drive\u0027]"},{"line_number":935,"context_line":"                                     else False)"},{"line_number":936,"context_line":"                build_request \u003d objects.BuildRequest(context,"},{"line_number":937,"context_line":"                        request_spec\u003dreq_spec,"},{"line_number":938,"context_line":"                        project_id\u003dcontext.project_id,"}],"source_content_type":"text/x-python","patch_set":12,"id":"7af24918_c3a4afa7","line":935,"range":{"start_line":934,"start_character":16,"end_line":935,"end_character":48},"updated":"2016-03-02 20:33:28.000000000","message":"We should probably use strutils.bool_from_string from oslo.utils here.","commit_id":"e7020d130b1401cee84aaf80287dcd4be9feea89"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b62cae543812a96f4724b0355d670a520a6b5a31","unresolved":false,"context_lines":[{"line_number":931,"context_line":"                # NOTE: base_options[\u0027config_drive\u0027] is either True or \u0027\u0027 due"},{"line_number":932,"context_line":"                # to how it\u0027s represented in the instances table in the db."},{"line_number":933,"context_line":"                # BuildRequest needs a boolean."},{"line_number":934,"context_line":"                bool_config_drive \u003d (True if base_options[\u0027config_drive\u0027]"},{"line_number":935,"context_line":"                                     else False)"},{"line_number":936,"context_line":"                build_request \u003d objects.BuildRequest(context,"},{"line_number":937,"context_line":"                        request_spec\u003dreq_spec,"},{"line_number":938,"context_line":"                        project_id\u003dcontext.project_id,"}],"source_content_type":"text/x-python","patch_set":12,"id":"7af24918_f4308303","line":935,"range":{"start_line":934,"start_character":16,"end_line":935,"end_character":48},"in_reply_to":"7af24918_34eaeb0f","updated":"2016-03-02 21:32:05.000000000","message":"OK. Looking at the json schema validation on config_drive:\n\nhttps://github.com/openstack/nova/blob/master/nova/api/openstack/compute/schemas/config_drive.py#L18\n\nIt looks like we should get valid boolean values:\n\nhttps://github.com/openstack/nova/blob/master/nova/api/validation/parameter_types.py#L154-L160","commit_id":"e7020d130b1401cee84aaf80287dcd4be9feea89"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"9000eaaf60949670760b8f1022823da88a40c37f","unresolved":false,"context_lines":[{"line_number":931,"context_line":"                # NOTE: base_options[\u0027config_drive\u0027] is either True or \u0027\u0027 due"},{"line_number":932,"context_line":"                # to how it\u0027s represented in the instances table in the db."},{"line_number":933,"context_line":"                # BuildRequest needs a boolean."},{"line_number":934,"context_line":"                bool_config_drive \u003d (True if base_options[\u0027config_drive\u0027]"},{"line_number":935,"context_line":"                                     else False)"},{"line_number":936,"context_line":"                build_request \u003d objects.BuildRequest(context,"},{"line_number":937,"context_line":"                        request_spec\u003dreq_spec,"},{"line_number":938,"context_line":"                        project_id\u003dcontext.project_id,"}],"source_content_type":"text/x-python","patch_set":12,"id":"7af24918_34eaeb0f","line":935,"range":{"start_line":934,"start_character":16,"end_line":935,"end_character":48},"in_reply_to":"7af24918_c3a4afa7","updated":"2016-03-02 21:19:07.000000000","message":"I\u0027ll use it, but it isn\u0027t really meant for this situation. There\u0027s no string with \u0027False\u0027, \u0027f\u0027, \u0027false\u0027 or any other boolean indicator here. It\u0027s basically True if base_options[\u0027config_drive\u0027] is True and False if anything else. base_options[\u0027config_drive\u0027] could be \u0027t\u0027 and we should still consider it False here.","commit_id":"e7020d130b1401cee84aaf80287dcd4be9feea89"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"713a2f053d7fa7b30d2a19bb7e3301801e27db3c","unresolved":false,"context_lines":[{"line_number":937,"context_line":"                        request_spec\u003dreq_spec,"},{"line_number":938,"context_line":"                        project_id\u003dcontext.project_id,"},{"line_number":939,"context_line":"                        user_id\u003dcontext.user_id,"},{"line_number":940,"context_line":"                        display_name\u003dbase_options[\u0027display_name\u0027],"},{"line_number":941,"context_line":"                        instance_metadata\u003dbase_options[\u0027metadata\u0027],"},{"line_number":942,"context_line":"                        progress\u003d0,"},{"line_number":943,"context_line":"                        vm_state\u003dvm_states.BUILDING,"}],"source_content_type":"text/x-python","patch_set":12,"id":"7af24918_5e807846","line":940,"range":{"start_line":940,"start_character":24,"end_line":940,"end_character":66},"updated":"2016-03-02 20:38:58.000000000","message":"As noted in IRC, looks like we should be using _populate_instance_names here.","commit_id":"e7020d130b1401cee84aaf80287dcd4be9feea89"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1f5b5185636122c5cbf48acef9f88e7f69473786","unresolved":false,"context_lines":[{"line_number":937,"context_line":"                        request_spec\u003dreq_spec,"},{"line_number":938,"context_line":"                        project_id\u003dcontext.project_id,"},{"line_number":939,"context_line":"                        user_id\u003dcontext.user_id,"},{"line_number":940,"context_line":"                        display_name\u003dbase_options[\u0027display_name\u0027],"},{"line_number":941,"context_line":"                        instance_metadata\u003dbase_options[\u0027metadata\u0027],"},{"line_number":942,"context_line":"                        progress\u003d0,"},{"line_number":943,"context_line":"                        vm_state\u003dvm_states.BUILDING,"}],"source_content_type":"text/x-python","patch_set":12,"id":"7af24918_be737c7f","line":940,"range":{"start_line":940,"start_character":24,"end_line":940,"end_character":66},"in_reply_to":"7af24918_5e807846","updated":"2016-03-02 20:41:59.000000000","message":"And this I guess? _apply_instance_name_template","commit_id":"e7020d130b1401cee84aaf80287dcd4be9feea89"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b61c02c768cf99cd3302b9b4dd6c11312df0a27b","unresolved":false,"context_lines":[{"line_number":1393,"context_line":""},{"line_number":1394,"context_line":"        instance.system_metadata.update(system_meta)"},{"line_number":1395,"context_line":""},{"line_number":1396,"context_line":"        pop_sec_groups \u003d self.security_group_api.populate_security_groups"},{"line_number":1397,"context_line":"        instance.security_groups \u003d pop_sec_groups(security_groups)"},{"line_number":1398,"context_line":""},{"line_number":1399,"context_line":"        return instance"},{"line_number":1400,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"7af24918_43753f96","line":1397,"range":{"start_line":1396,"start_character":8,"end_line":1397,"end_character":66},"updated":"2016-03-02 20:33:28.000000000","message":"Can we pass the populated security groups down from the _provision_instances method so we don\u0027t have to call the security group API twice to do this (thinking about redundant round trips to neutron).","commit_id":"e7020d130b1401cee84aaf80287dcd4be9feea89"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"91457d27e84b72f4fb339675a38ddabe63cb0125","unresolved":false,"context_lines":[{"line_number":1419,"context_line":"    # NOTE(bcwaldon): No policy check since this is only used by scheduler and"},{"line_number":1420,"context_line":"    # the compute api. That should probably be cleaned up, though."},{"line_number":1421,"context_line":"    def create_db_entry_for_new_instance(self, context, instance_type, image,"},{"line_number":1422,"context_line":"            instance, security_group, block_device_mapping, num_instances,"},{"line_number":1423,"context_line":"            index, shutdown_terminate\u003dFalse):"},{"line_number":1424,"context_line":"        \"\"\"Create an entry in the DB for this new instance,"},{"line_number":1425,"context_line":"        including any related table updates (such as security group,"}],"source_content_type":"text/x-python","patch_set":13,"id":"7af24918_154d23c4","line":1422,"range":{"start_line":1422,"start_character":22,"end_line":1422,"end_character":36},"updated":"2016-03-03 00:35:38.000000000","message":"So now this is the populated security groups list from the security group API. Which should be fine. But this is a public method and it\u0027s used by the cells v1 scheduler code:\n\nhttps://github.com/openstack/nova/blob/master/nova/cells/scheduler.py#L98\n\nWhich is not passing in the converted (populated) security group list, so....we\u0027d have to check the type in _populate_instance_for_create, or change the cells scheduler code to call the security group API to convert, or just leave the conversion call in _populate_instance_for_create, which for nova-network would be a no-op and in the neutron case it would just return a new empty SecurityGroupList.","commit_id":"88ae3768b399e895f1b0b67aefd29a46af57c8f2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"42efab9772f3bec1d4eaacebba2109b03fadabb7","unresolved":false,"context_lines":[{"line_number":1419,"context_line":"    # NOTE(bcwaldon): No policy check since this is only used by scheduler and"},{"line_number":1420,"context_line":"    # the compute api. That should probably be cleaned up, though."},{"line_number":1421,"context_line":"    def create_db_entry_for_new_instance(self, context, instance_type, image,"},{"line_number":1422,"context_line":"            instance, security_group, block_device_mapping, num_instances,"},{"line_number":1423,"context_line":"            index, shutdown_terminate\u003dFalse):"},{"line_number":1424,"context_line":"        \"\"\"Create an entry in the DB for this new instance,"},{"line_number":1425,"context_line":"        including any related table updates (such as security group,"}],"source_content_type":"text/x-python","patch_set":13,"id":"7af24918_584fe26a","line":1422,"range":{"start_line":1422,"start_character":22,"end_line":1422,"end_character":36},"in_reply_to":"7af24918_154d23c4","updated":"2016-03-03 00:37:07.000000000","message":"Yeah cells failed:\n\nhttp://logs.openstack.org/25/278125/13/check/gate-tempest-dsvm-cells/f68f46f/logs/screen-n-cell-child.txt.gz?level\u003dTRACE#_2016-03-02_23_51_54_049\n\nSo we probably need to leave that conversion method in place, would always add a TODO to clean it up later.","commit_id":"88ae3768b399e895f1b0b67aefd29a46af57c8f2"}],"nova/tests/unit/compute/test_compute_api.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e1e8c1273be6fa2c7122dc473a9ed253308a56b1","unresolved":false,"context_lines":[{"line_number":2929,"context_line":""},{"line_number":2930,"context_line":"        do_test()"},{"line_number":2931,"context_line":""},{"line_number":2932,"context_line":"    def test_provision_instances_creates_destroys_build_request(self):"},{"line_number":2933,"context_line":"        @mock.patch.object(self.compute_api, \u0027_check_num_instances_quota\u0027)"},{"line_number":2934,"context_line":"        @mock.patch.object(objects.Instance, \u0027create\u0027)"},{"line_number":2935,"context_line":"        @mock.patch.object(self.compute_api.security_group_api,"}],"source_content_type":"text/x-python","patch_set":12,"id":"7af24918_99acda36","line":2932,"updated":"2016-03-02 21:11:42.000000000","message":"Might just be me, but rather than duplicating basically the entire test above just to add the BuildRequest mocks for create/destroy, it would seem easier to just add that to the existing test.","commit_id":"e7020d130b1401cee84aaf80287dcd4be9feea89"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b62cae543812a96f4724b0355d670a520a6b5a31","unresolved":false,"context_lines":[{"line_number":2929,"context_line":""},{"line_number":2930,"context_line":"        do_test()"},{"line_number":2931,"context_line":""},{"line_number":2932,"context_line":"    def test_provision_instances_creates_destroys_build_request(self):"},{"line_number":2933,"context_line":"        @mock.patch.object(self.compute_api, \u0027_check_num_instances_quota\u0027)"},{"line_number":2934,"context_line":"        @mock.patch.object(objects.Instance, \u0027create\u0027)"},{"line_number":2935,"context_line":"        @mock.patch.object(self.compute_api.security_group_api,"}],"source_content_type":"text/x-python","patch_set":12,"id":"7af24918_6e077f18","line":2932,"in_reply_to":"7af24918_545cd7ec","updated":"2016-03-02 21:32:05.000000000","message":"OK, those kinds of tests are equally gross, so I guess whatever makes the most sense and is maintainable.","commit_id":"e7020d130b1401cee84aaf80287dcd4be9feea89"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"3a8d6413e849c52c92d13d646c4f39fc0838d2ae","unresolved":false,"context_lines":[{"line_number":2929,"context_line":""},{"line_number":2930,"context_line":"        do_test()"},{"line_number":2931,"context_line":""},{"line_number":2932,"context_line":"    def test_provision_instances_creates_destroys_build_request(self):"},{"line_number":2933,"context_line":"        @mock.patch.object(self.compute_api, \u0027_check_num_instances_quota\u0027)"},{"line_number":2934,"context_line":"        @mock.patch.object(objects.Instance, \u0027create\u0027)"},{"line_number":2935,"context_line":"        @mock.patch.object(self.compute_api.security_group_api,"}],"source_content_type":"text/x-python","patch_set":12,"id":"7af24918_dd2f6786","line":2932,"in_reply_to":"7af24918_6e077f18","updated":"2016-03-02 23:03:54.000000000","message":"I\u0027m going to leave this for now and try to clean it up in a followup.","commit_id":"e7020d130b1401cee84aaf80287dcd4be9feea89"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"9000eaaf60949670760b8f1022823da88a40c37f","unresolved":false,"context_lines":[{"line_number":2929,"context_line":""},{"line_number":2930,"context_line":"        do_test()"},{"line_number":2931,"context_line":""},{"line_number":2932,"context_line":"    def test_provision_instances_creates_destroys_build_request(self):"},{"line_number":2933,"context_line":"        @mock.patch.object(self.compute_api, \u0027_check_num_instances_quota\u0027)"},{"line_number":2934,"context_line":"        @mock.patch.object(objects.Instance, \u0027create\u0027)"},{"line_number":2935,"context_line":"        @mock.patch.object(self.compute_api.security_group_api,"}],"source_content_type":"text/x-python","patch_set":12,"id":"7af24918_545cd7ec","line":2932,"in_reply_to":"7af24918_99acda36","updated":"2016-03-02 21:19:07.000000000","message":"Let me see about passing mocks out of a utility method. If I can\u0027t make that work this will end up with a bunch of if/else statements verifying different things for different test runs.","commit_id":"e7020d130b1401cee84aaf80287dcd4be9feea89"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e1e8c1273be6fa2c7122dc473a9ed253308a56b1","unresolved":false,"context_lines":[{"line_number":2951,"context_line":""},{"line_number":2952,"context_line":"            ctxt \u003d context.RequestContext(\u0027fake-user\u0027, \u0027fake-project\u0027)"},{"line_number":2953,"context_line":"            flavor \u003d self._create_flavor()"},{"line_number":2954,"context_line":"            min_count \u003d max_count \u003d 1"},{"line_number":2955,"context_line":"            boot_meta \u003d {"},{"line_number":2956,"context_line":"                \u0027id\u0027: \u0027fake-image-id\u0027,"},{"line_number":2957,"context_line":"                \u0027properties\u0027: {\u0027mappings\u0027: []},"}],"source_content_type":"text/x-python","patch_set":12,"id":"7af24918_194e0a96","line":2954,"range":{"start_line":2954,"start_character":12,"end_line":2954,"end_character":37},"updated":"2016-03-02 21:11:42.000000000","message":"Given the multiple instances case with display_name, it would probably be good to have a test that hits this code with more than one instance.","commit_id":"e7020d130b1401cee84aaf80287dcd4be9feea89"}]}