)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c93c3bfd2fe2247a525240a938c4c9e101cbfc91","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If an instance fails to successfully pass the quota recheck, it will"},{"line_number":10,"context_line":"raise a TooManyInstances exception, however, it will not hit the"},{"line_number":11,"context_line":"code which saves the instance mapping, leaving an instance with no"},{"line_number":12,"context_line":"assigned cell in the mapping table."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"This patch adds a test to make sure that an instance has the correct"},{"line_number":15,"context_line":"cell mapping if it fails in the quota recheck phase.  In addition, it"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"7f287b81_48448047","line":12,"range":{"start_line":11,"start_character":39,"end_line":12,"end_character":35},"updated":"2017-09-06 21:35:53.000000000","message":"And no BuildRequest, because _cleanup_build_artifacts removes the build request, that was a missing piece for me when I originally looked at this.\n\nJust a note to other reviewers.","commit_id":"396a77fd123937e935fcea03d7198f569dce0684"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5dfc66a5eca1a8c05552df232b5cea0ca6463811","unresolved":false,"context_lines":[{"line_number":17,"context_line":"uses the cell_mapping_cache dictionary to set the correct cell"},{"line_number":18,"context_line":"mapping before marking the instance as ERROR."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Co-Authored-By: Dan Smith \u003cdansmith@redhat.com\u003e"},{"line_number":21,"context_line":"Co-Authored-By: Matt Riedemann \u003cmriedem.os@gmail.com\u003e"},{"line_number":22,"context_line":"Co-Authored-By: melanie witt \u003cmelwittt@gmail.com\u003e"},{"line_number":23,"context_line":"Closes-Bug: #1715462"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Change-Id: I7ecb5feb47a5f358cd51bde87b75a3a6141b5b12"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"7f287b81_4867e0b8","line":22,"range":{"start_line":20,"start_character":0,"end_line":22,"end_character":49},"updated":"2017-09-06 21:43:25.000000000","message":"This must have been a real team effort.","commit_id":"db7e1cb742524e372860d36907e528326ba66a64"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e5a1c3fae1eca68fb97b6c5e4d356c3eea6c114f","unresolved":false,"context_lines":[{"line_number":17,"context_line":"uses the cell_mapping_cache dictionary to set the correct cell"},{"line_number":18,"context_line":"mapping before marking the instance as ERROR."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Co-Authored-By: Dan Smith \u003cdansmith@redhat.com\u003e"},{"line_number":21,"context_line":"Co-Authored-By: Matt Riedemann \u003cmriedem.os@gmail.com\u003e"},{"line_number":22,"context_line":"Co-Authored-By: melanie witt \u003cmelwittt@gmail.com\u003e"},{"line_number":23,"context_line":"Closes-Bug: #1715462"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"7f287b81_1d96a63f","line":20,"updated":"2017-09-07 13:41:59.000000000","message":"Heh, thanks for the nod but you deserve all the credit.","commit_id":"bf0f5d475458a67a12000ff49a8c5285c3ac9e45"}],"nova/conductor/manager.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"52d7243251a707375ffe8a3a3afb9ab25dec0226","unresolved":false,"context_lines":[{"line_number":481,"context_line":"        # be updated before this is destroyed."},{"line_number":482,"context_line":"        build_request.destroy()"},{"line_number":483,"context_line":""},{"line_number":484,"context_line":"    def _populate_instance_mapping(self, context, instance, host\u003dNone,"},{"line_number":485,"context_line":"                                   host_mapping\u003dNone):"},{"line_number":486,"context_line":"        try:"},{"line_number":487,"context_line":"            inst_mapping \u003d objects.InstanceMapping.get_by_instance_uuid("},{"line_number":488,"context_line":"                    context, instance.uuid)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f287b81_936a9eb0","line":485,"range":{"start_line":484,"start_character":60,"end_line":485,"end_character":52},"updated":"2017-09-06 20:33:32.000000000","message":"Can we add a docstring for these since I\u0027ve already been confused about when these are used or not, and what they are.","commit_id":"09f24306ba377275bf4f5375111c3410e09e70f7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bc3468fb05640a88c539242706bf9ea8b56a9b3d","unresolved":false,"context_lines":[{"line_number":499,"context_line":"            try:"},{"line_number":500,"context_line":"                if host_mapping is None:"},{"line_number":501,"context_line":"                    host_mapping \u003d objects.HostMapping.get_by_host(context,"},{"line_number":502,"context_line":"                            host[\u0027host\u0027])"},{"line_number":503,"context_line":"            except exception.HostMappingNotFound:"},{"line_number":504,"context_line":"                # NOTE(alaski): For now this exception means that a"},{"line_number":505,"context_line":"                # deployment has not migrated to cellsv2 and we should"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f287b81_786a711a","line":502,"range":{"start_line":502,"start_character":28,"end_line":502,"end_character":32},"updated":"2017-09-06 20:24:51.000000000","message":"What if host is None here?","commit_id":"09f24306ba377275bf4f5375111c3410e09e70f7"},{"author":{"_account_id":1004,"name":"Mohammed Naser","email":"mnaser@vexxhost.com","username":"mnaser"},"change_message_id":"86f40cd415b94acdffc243bc2af3784f4d682347","unresolved":false,"context_lines":[{"line_number":499,"context_line":"            try:"},{"line_number":500,"context_line":"                if host_mapping is None:"},{"line_number":501,"context_line":"                    host_mapping \u003d objects.HostMapping.get_by_host(context,"},{"line_number":502,"context_line":"                            host[\u0027host\u0027])"},{"line_number":503,"context_line":"            except exception.HostMappingNotFound:"},{"line_number":504,"context_line":"                # NOTE(alaski): For now this exception means that a"},{"line_number":505,"context_line":"                # deployment has not migrated to cellsv2 and we should"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f287b81_d37a6675","line":502,"range":{"start_line":502,"start_character":28,"end_line":502,"end_character":32},"in_reply_to":"7f287b81_786a711a","updated":"2017-09-06 20:37:03.000000000","message":"There was no handling for host being set to None in the original codebase, so it was assumed that it would never be sent as None.\n\nI added host\u003dNone in order to give the ability to provide a host_mapping directly *or* a host if no host mapping is available.\n\nThis function was referenced one other location in the same file and called with context, instance and host.  Therefore, every call to this function will either get a host (in the case of the original call) or host mapping.","commit_id":"09f24306ba377275bf4f5375111c3410e09e70f7"},{"author":{"_account_id":1004,"name":"Mohammed Naser","email":"mnaser@vexxhost.com","username":"mnaser"},"change_message_id":"86f40cd415b94acdffc243bc2af3784f4d682347","unresolved":false,"context_lines":[{"line_number":595,"context_line":"                # If this is a reschedule the instance is already mapped to"},{"line_number":596,"context_line":"                # this cell and the BuildRequest is already deleted so ignore"},{"line_number":597,"context_line":"                # the logic below."},{"line_number":598,"context_line":"                inst_mapping \u003d self._populate_instance_mapping(context,"},{"line_number":599,"context_line":"                                                               instance,"},{"line_number":600,"context_line":"                                                               host)"},{"line_number":601,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f287b81_934bbe29","line":598,"updated":"2017-09-06 20:37:03.000000000","message":"The existing call to `_populate_instance_mapping`","commit_id":"09f24306ba377275bf4f5375111c3410e09e70f7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bc3468fb05640a88c539242706bf9ea8b56a9b3d","unresolved":false,"context_lines":[{"line_number":990,"context_line":"                with obj_target_cell(instance, cell):"},{"line_number":991,"context_line":"                    instance.create()"},{"line_number":992,"context_line":"                    instances.append(instance)"},{"line_number":993,"context_line":"                self._populate_instance_mapping(context, instance,"},{"line_number":994,"context_line":"                                                host_mapping\u003dhost_mapping)"},{"line_number":995,"context_line":""},{"line_number":996,"context_line":"        # NOTE(melwitt): We recheck the quota after creating the"},{"line_number":997,"context_line":"        # objects to prevent users from allocating more resources"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f287b81_f8cc2136","line":994,"range":{"start_line":993,"start_character":48,"end_line":994,"end_character":73},"updated":"2017-09-06 20:24:51.000000000","message":"You\u0027re not providing a host variable.","commit_id":"09f24306ba377275bf4f5375111c3410e09e70f7"},{"author":{"_account_id":1004,"name":"Mohammed Naser","email":"mnaser@vexxhost.com","username":"mnaser"},"change_message_id":"86f40cd415b94acdffc243bc2af3784f4d682347","unresolved":false,"context_lines":[{"line_number":990,"context_line":"                with obj_target_cell(instance, cell):"},{"line_number":991,"context_line":"                    instance.create()"},{"line_number":992,"context_line":"                    instances.append(instance)"},{"line_number":993,"context_line":"                self._populate_instance_mapping(context, instance,"},{"line_number":994,"context_line":"                                                host_mapping\u003dhost_mapping)"},{"line_number":995,"context_line":""},{"line_number":996,"context_line":"        # NOTE(melwitt): We recheck the quota after creating the"},{"line_number":997,"context_line":"        # objects to prevent users from allocating more resources"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f287b81_785dd145","line":994,"range":{"start_line":993,"start_character":48,"end_line":994,"end_character":73},"in_reply_to":"7f287b81_f8cc2136","updated":"2017-09-06 20:37:03.000000000","message":"The reason behind why I did not provide host is that it would have introduced another database call by calling `objects.HostMapping.get_by_host`.\n\nI figured by passing the host_mapping which could be retrieved from host_mapping_cache, we save the extra call to retrieve something that we\u0027ve already retrieved.","commit_id":"09f24306ba377275bf4f5375111c3410e09e70f7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"52d7243251a707375ffe8a3a3afb9ab25dec0226","unresolved":false,"context_lines":[{"line_number":990,"context_line":"                with obj_target_cell(instance, cell):"},{"line_number":991,"context_line":"                    instance.create()"},{"line_number":992,"context_line":"                    instances.append(instance)"},{"line_number":993,"context_line":"                self._populate_instance_mapping(context, instance,"},{"line_number":994,"context_line":"                                                host_mapping\u003dhost_mapping)"},{"line_number":995,"context_line":""},{"line_number":996,"context_line":"        # NOTE(melwitt): We recheck the quota after creating the"},{"line_number":997,"context_line":"        # objects to prevent users from allocating more resources"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f287b81_d36ca6d1","line":994,"range":{"start_line":993,"start_character":48,"end_line":994,"end_character":73},"in_reply_to":"7f287b81_f8cc2136","updated":"2017-09-06 20:33:32.000000000","message":"Well, I guess the idea is that if host_mapping is provided, then host isn\u0027t used, so that\u0027s probably OK.","commit_id":"09f24306ba377275bf4f5375111c3410e09e70f7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1d4357260d7a310662505457ef8b816abafd4354","unresolved":false,"context_lines":[{"line_number":1028,"context_line":"                objects.InstanceAction.action_start("},{"line_number":1029,"context_line":"                    cctxt, instance.uuid, instance_actions.CREATE,"},{"line_number":1030,"context_line":"                    want_result\u003dFalse)"},{"line_number":1031,"context_line":"                instance_bdms \u003d self._create_block_device_mapping("},{"line_number":1032,"context_line":"                    cell, instance.flavor, instance.uuid, block_device_mapping)"},{"line_number":1033,"context_line":"                instance_tags \u003d self._create_tags(cctxt, instance.uuid, tags)"},{"line_number":1034,"context_line":""},{"line_number":1035,"context_line":"            # TODO(Kevin Zheng): clean this up once instance.create() handles"},{"line_number":1036,"context_line":"            # tags; we do this so the instance.create notification in"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f287b81_1300ee00","line":1033,"range":{"start_line":1031,"start_character":16,"end_line":1033,"end_character":77},"updated":"2017-09-06 20:31:04.000000000","message":"I don\u0027t know if the mapping should exist before we have the BDMs and tags created in the cell. If the mapping exists, we\u0027ll pull that from the cell but we\u0027re going to be missing any bdm/tag information for that instance in the cell until we get here. If the mapping didn\u0027t exist, I think we\u0027d pull it out of the build request object (how it works before this change).","commit_id":"09f24306ba377275bf4f5375111c3410e09e70f7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"17fb52f33ea308f5f70571e5e9be3e558fb487b8","unresolved":false,"context_lines":[{"line_number":1014,"context_line":"                    context, instance.flavor, 0, 0,"},{"line_number":1015,"context_line":"                    orig_num_req\u003dlen(build_requests))"},{"line_number":1016,"context_line":"            except exception.TooManyInstances as exc:"},{"line_number":1017,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":1018,"context_line":"                    self._cleanup_build_artifacts(context, exc, instances,"},{"line_number":1019,"context_line":"                                                  build_requests,"},{"line_number":1020,"context_line":"                                                  request_specs)"},{"line_number":1021,"context_line":""},{"line_number":1022,"context_line":"        for (build_request, request_spec, host, instance) in six.moves.zip("},{"line_number":1023,"context_line":"                build_requests, request_specs, hosts, instances):"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f287b81_45710944","line":1020,"range":{"start_line":1017,"start_character":16,"end_line":1020,"end_character":64},"updated":"2017-09-06 21:09:51.000000000","message":"To recap the discussion from IRC, let\u0027s just map the instances here and we can backport that simpler change. And then in Queens, we\u0027ll combine the two for loops and move the quota check to the end, which is the more complicated change.","commit_id":"b25f9c59007bd58cc9b69ce846c4226be3c774a9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"436da32de010be7b103d42faf187b47ef55985eb","unresolved":false,"context_lines":[{"line_number":1014,"context_line":"                    context, instance.flavor, 0, 0,"},{"line_number":1015,"context_line":"                    orig_num_req\u003dlen(build_requests))"},{"line_number":1016,"context_line":"            except exception.TooManyInstances as exc:"},{"line_number":1017,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":1018,"context_line":"                    self._cleanup_build_artifacts(context, exc, instances,"},{"line_number":1019,"context_line":"                                                  build_requests,"},{"line_number":1020,"context_line":"                                                  request_specs)"},{"line_number":1021,"context_line":""},{"line_number":1022,"context_line":"        for (build_request, request_spec, host, instance) in six.moves.zip("},{"line_number":1023,"context_line":"                build_requests, request_specs, hosts, instances):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_39eab90d","line":1020,"range":{"start_line":1017,"start_character":16,"end_line":1020,"end_character":64},"in_reply_to":"7f287b81_45710944","updated":"2018-07-27 21:59:35.000000000","message":"http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2017-09-06.log.html#t2017-09-06T20:33:51","commit_id":"b25f9c59007bd58cc9b69ce846c4226be3c774a9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c93c3bfd2fe2247a525240a938c4c9e101cbfc91","unresolved":false,"context_lines":[{"line_number":1089,"context_line":""},{"line_number":1090,"context_line":"            # TODO(mnaser): The cell mapping should already be populated by"},{"line_number":1091,"context_line":"            #               this point to avoid setting it below here."},{"line_number":1092,"context_line":"            for instance in instances:"},{"line_number":1093,"context_line":"                inst_mapping \u003d objects.InstanceMapping.get_by_instance_uuid("},{"line_number":1094,"context_line":"                    context, instance.uuid)"},{"line_number":1095,"context_line":"                inst_mapping.cell_mapping \u003d cell_mapping_cache[instance.uuid]"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f287b81_08a1d8f8","line":1092,"range":{"start_line":1092,"start_character":12,"end_line":1092,"end_character":38},"updated":"2017-09-06 21:35:53.000000000","message":"You don\u0027t want the loop here because some of these instances are None. We\u0027re already looping over instances, and have skipped one if it was None, so you just need to handle the instance in scope already.","commit_id":"396a77fd123937e935fcea03d7198f569dce0684"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"954bcf5315749e3aa13f6898a521987930cafc93","unresolved":false,"context_lines":[{"line_number":1046,"context_line":"            inst_mapping.cell_mapping \u003d cell"},{"line_number":1047,"context_line":"            inst_mapping.save()"},{"line_number":1048,"context_line":""},{"line_number":1049,"context_line":"            if not self._delete_build_request("},{"line_number":1050,"context_line":"                    context, build_request, instance, cell, instance_bdms,"},{"line_number":1051,"context_line":"                    instance_tags):"},{"line_number":1052,"context_line":"                # The build request was deleted before/during scheduling so"}],"source_content_type":"text/x-python","patch_set":8,"id":"7f287b81_f20f9285","line":1049,"updated":"2017-09-07 05:43:07.000000000","message":"This isn\u0027t a question for this patch also, I\u0027m thinking this didn\u0027t clean up the resource allocation in the placement? I need to check that","commit_id":"80e79df3d7f4ad35cd04d7dcc7df8e81f9b0d7ba"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e73c80cccf0db09d0f60013403e72fa4019c311b","unresolved":false,"context_lines":[{"line_number":1046,"context_line":"            inst_mapping.cell_mapping \u003d cell"},{"line_number":1047,"context_line":"            inst_mapping.save()"},{"line_number":1048,"context_line":""},{"line_number":1049,"context_line":"            if not self._delete_build_request("},{"line_number":1050,"context_line":"                    context, build_request, instance, cell, instance_bdms,"},{"line_number":1051,"context_line":"                    instance_tags):"},{"line_number":1052,"context_line":"                # The build request was deleted before/during scheduling so"}],"source_content_type":"text/x-python","patch_set":8,"id":"7f287b81_fd42ba0e","line":1049,"in_reply_to":"7f287b81_f20f9285","updated":"2017-09-07 13:36:41.000000000","message":"It depends on where / when the build request was deleted and if we got past scheduling. But yeah at this point we\u0027ve gotten hosts from the scheduler which means allocations were made on those hosts, and we\u0027ve created an instance in the cell, but the instance doesn\u0027t have a host value set yet, so even if the instance was deleted in the API while building, it wouldn\u0027t have a host to cast to for the deletion on the compute service which would cleanup the allocation. So I\u0027m thinking this might fall under bug 1679750.","commit_id":"80e79df3d7f4ad35cd04d7dcc7df8e81f9b0d7ba"}]}