)]}'
{"nova/compute/manager.py":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"b307351a3faf606ff7eb417134aeef613014a793","unresolved":false,"context_lines":[{"line_number":654,"context_line":"            \u0027status\u0027: [\u0027accepted\u0027, \u0027pre-migrating\u0027, \u0027done\u0027],"},{"line_number":655,"context_line":"            \u0027migration_type\u0027: \u0027evacuation\u0027,"},{"line_number":656,"context_line":"        }"},{"line_number":657,"context_line":"        with utils.temporary_mutation(context, read_deleted\u003d\u0027yes\u0027):"},{"line_number":658,"context_line":"            evacuations \u003d objects.MigrationList.get_by_filters(context,"},{"line_number":659,"context_line":"                                                               filters)"},{"line_number":660,"context_line":"        if not evacuations:"},{"line_number":661,"context_line":"            return"},{"line_number":662,"context_line":"        evacuations \u003d {mig.instance_uuid: mig for mig in evacuations}"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_79d7db50","line":659,"range":{"start_line":657,"start_character":0,"end_line":659,"end_character":71},"updated":"2018-09-28 16:14:57.000000000","message":"Suggestion: given that you\u0027re creating read_deleted_context below you could also use it here.","commit_id":"1838fcde1535aae34b0647bb8b03208d58b3aa36"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"cfc0edcce4057143d249e32a159a0c4e52f4b98f","unresolved":false,"context_lines":[{"line_number":654,"context_line":"            \u0027status\u0027: [\u0027accepted\u0027, \u0027pre-migrating\u0027, \u0027done\u0027],"},{"line_number":655,"context_line":"            \u0027migration_type\u0027: \u0027evacuation\u0027,"},{"line_number":656,"context_line":"        }"},{"line_number":657,"context_line":"        with utils.temporary_mutation(context, read_deleted\u003d\u0027yes\u0027):"},{"line_number":658,"context_line":"            evacuations \u003d objects.MigrationList.get_by_filters(context,"},{"line_number":659,"context_line":"                                                               filters)"},{"line_number":660,"context_line":"        if not evacuations:"},{"line_number":661,"context_line":"            return"},{"line_number":662,"context_line":"        evacuations \u003d {mig.instance_uuid: mig for mig in evacuations}"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_e651ca0e","line":659,"range":{"start_line":657,"start_character":0,"end_line":659,"end_character":71},"in_reply_to":"3f79a3b5_6ed870ef","updated":"2018-10-02 14:07:57.000000000","message":"Would the fact that you would be changing to an admin context affect the result?\n\nOr are you already admin? In which case, why not use temporary_mutation (which is much cheaper) below?","commit_id":"1838fcde1535aae34b0647bb8b03208d58b3aa36"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4e61d93c0e40920cc2c13f3d74be19b893a149b6","unresolved":false,"context_lines":[{"line_number":654,"context_line":"            \u0027status\u0027: [\u0027accepted\u0027, \u0027pre-migrating\u0027, \u0027done\u0027],"},{"line_number":655,"context_line":"            \u0027migration_type\u0027: \u0027evacuation\u0027,"},{"line_number":656,"context_line":"        }"},{"line_number":657,"context_line":"        with utils.temporary_mutation(context, read_deleted\u003d\u0027yes\u0027):"},{"line_number":658,"context_line":"            evacuations \u003d objects.MigrationList.get_by_filters(context,"},{"line_number":659,"context_line":"                                                               filters)"},{"line_number":660,"context_line":"        if not evacuations:"},{"line_number":661,"context_line":"            return"},{"line_number":662,"context_line":"        evacuations \u003d {mig.instance_uuid: mig for mig in evacuations}"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_6ed870ef","line":659,"range":{"start_line":657,"start_character":0,"end_line":659,"end_character":71},"in_reply_to":"3f79a3b5_79d7db50","updated":"2018-09-28 18:04:02.000000000","message":"Yeah I thought about that, figured I\u0027d wait to see if people really felt strongly about it.","commit_id":"1838fcde1535aae34b0647bb8b03208d58b3aa36"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"148ece9546902f45c29e078694f6d6cf2720b193","unresolved":false,"context_lines":[{"line_number":654,"context_line":"            \u0027status\u0027: [\u0027accepted\u0027, \u0027pre-migrating\u0027, \u0027done\u0027],"},{"line_number":655,"context_line":"            \u0027migration_type\u0027: \u0027evacuation\u0027,"},{"line_number":656,"context_line":"        }"},{"line_number":657,"context_line":"        with utils.temporary_mutation(context, read_deleted\u003d\u0027yes\u0027):"},{"line_number":658,"context_line":"            evacuations \u003d objects.MigrationList.get_by_filters(context,"},{"line_number":659,"context_line":"                                                               filters)"},{"line_number":660,"context_line":"        if not evacuations:"},{"line_number":661,"context_line":"            return"},{"line_number":662,"context_line":"        evacuations \u003d {mig.instance_uuid: mig for mig in evacuations}"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_b7d43b20","line":659,"range":{"start_line":657,"start_character":0,"end_line":659,"end_character":71},"in_reply_to":"3f79a3b5_e651ca0e","updated":"2018-10-03 18:28:37.000000000","message":"\u003e Would the fact that you would be changing to an admin context affect the result?\n\nWe\u0027re already admin context, see L1204.\n\n\u003e In which case, why not use temporary_mutation (which is much cheaper) below?\n\nBecause we need the Instance._context to have read_deleted\u003d\u0027yes\u0027 so it can\u0027t be temporary. Otherwise you\u0027d still potentially get InstanceNotFound when lazy-loading the instance.flavor field on a deleted instance.","commit_id":"1838fcde1535aae34b0647bb8b03208d58b3aa36"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"cfc0edcce4057143d249e32a159a0c4e52f4b98f","unresolved":false,"context_lines":[{"line_number":715,"context_line":"            # If the instance was deleted in the interim, assume its"},{"line_number":716,"context_line":"            # allocations were properly cleaned up (either by its hosting"},{"line_number":717,"context_line":"            # compute service or the API)."},{"line_number":718,"context_line":"            if (not instance.deleted and"},{"line_number":719,"context_line":"                    not scheduler_utils.remove_allocation_from_compute("},{"line_number":720,"context_line":"                        context, instance, cn_uuid, self.reportclient)):"},{"line_number":721,"context_line":"                LOG.error(\"Failed to clean allocation of evacuated instance \""},{"line_number":722,"context_line":"                          \"on the source node %s\","}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_bbfd2b3c","line":719,"range":{"start_line":718,"start_character":0,"end_line":719,"end_character":71},"updated":"2018-10-02 14:07:57.000000000","message":"Ugh.\n\nremove_allocation_from_compute \u003d\u003e remove_provider_from_instance_allocation funnels all sorts of different conditions into \"False\", which sucks.\n\nIMO this whole thing would be better if we could leave out the instance.deleted check and instead test remove_provider_from_instance_allocation for the 404 from the initial GET /allocations/{c}.\n\nCause if there\u0027s any path where we fail the allocation cleanup but still mark the instance deleted, this change will introduce a leak.\n\nUnless it\u0027s trivial to prove ^ impossible, perhaps it would be better to switch the conditions around. That is, attempt the allocation cleanup, but if it \"fails\", only emit the error log if the instance is not deleted.\n\nAFAICT, this change isn\u0027t actually part of the bug; it\u0027s just an optimization and avoiding a couple of log messages. If I\u0027m right, perhaps it would be best to split this into its own patch. (And maybe in that patch we can actually eviscerate remove_provider_from_instance_allocation, since we shouldn\u0027t need it anymore, since we should no longer be migrating between hosts that do the \"doubled allocation\" thing, right? (I get that this patch needs to be backportable to Pike, so that wouldn\u0027t be appropriate to do here.))","commit_id":"1838fcde1535aae34b0647bb8b03208d58b3aa36"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"148ece9546902f45c29e078694f6d6cf2720b193","unresolved":false,"context_lines":[{"line_number":715,"context_line":"            # If the instance was deleted in the interim, assume its"},{"line_number":716,"context_line":"            # allocations were properly cleaned up (either by its hosting"},{"line_number":717,"context_line":"            # compute service or the API)."},{"line_number":718,"context_line":"            if (not instance.deleted and"},{"line_number":719,"context_line":"                    not scheduler_utils.remove_allocation_from_compute("},{"line_number":720,"context_line":"                        context, instance, cn_uuid, self.reportclient)):"},{"line_number":721,"context_line":"                LOG.error(\"Failed to clean allocation of evacuated instance \""},{"line_number":722,"context_line":"                          \"on the source node %s\","}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_17cacf4d","line":719,"range":{"start_line":718,"start_character":0,"end_line":719,"end_character":71},"in_reply_to":"3f79a3b5_bbfd2b3c","updated":"2018-10-03 18:28:37.000000000","message":"\u003e That is, attempt the allocation cleanup, but if it \"fails\", only emit the error log if the instance is not deleted.\n\nremove_provider_from_instance_allocation will still log an error if the instance doesn\u0027t have any allocations:\n\nhttps://github.com/openstack/nova/blob/0163b9bfb54aaa89b0574c86e7fd36321eebccfe/nova/scheduler/client/report.py#L1866\n\n\u003e AFAICT, this change isn\u0027t actually part of the bug; it\u0027s just an optimization and avoiding a couple of log messages.\n\nThat is incorrect. It is precisely the bug, because the method we\u0027re calling is calling instance.flavor which will lazy-load the flavor on a deleted instance and raise InstanceNotFound:\n\nhttps://github.com/openstack/nova/blob/cce3208cc28268e4b50e155c205bcab9f1da2a4b/nova/scheduler/utils.py#L998\n\nThe reason I needed to load the instances with a read_deleted\u003d\u0027yes\u0027 context above is because some virt drivers will load fields off the instance in the driver.destroy() method, which I found after I wrote the functional test.","commit_id":"1838fcde1535aae34b0647bb8b03208d58b3aa36"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3e0d2625f24c813ee60d8cc9e494a6fa0b20725c","unresolved":false,"context_lines":[{"line_number":670,"context_line":"        read_deleted_context \u003d context.elevated(read_deleted\u003d\u0027yes\u0027)"},{"line_number":671,"context_line":"        # TODO(mriedem): We could optimize by pre-loading the joined fields"},{"line_number":672,"context_line":"        # we know we\u0027ll use, like info_cache and flavor."},{"line_number":673,"context_line":"        local_instances \u003d self._get_instances_on_driver(read_deleted_context)"},{"line_number":674,"context_line":"        evacuated \u003d [inst for inst in local_instances"},{"line_number":675,"context_line":"                     if inst.uuid in evacuations]"},{"line_number":676,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_ad53f514","line":673,"updated":"2018-10-23 16:57:19.000000000","message":"This is a whack-a-mole game, which I think we shouldn\u0027t play. Personally, if we have an instance (which we may have looked up as deleted), I think it\u0027s reasonable to expect that we will be able to lazy-load things from that instance without special handling (that\u0027s kindof the point of lazy-loading in general). So why not just do this and avoid having to whack all these moles?\n\nhttps://review.openstack.org/#/c/575190/","commit_id":"6be64ff6f4a87f74c4e321569bd1143ab394ca74"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fa4bba6c2c54c8ae286ef7dadd7ab7def2d212db","unresolved":false,"context_lines":[{"line_number":670,"context_line":"        read_deleted_context \u003d context.elevated(read_deleted\u003d\u0027yes\u0027)"},{"line_number":671,"context_line":"        # TODO(mriedem): We could optimize by pre-loading the joined fields"},{"line_number":672,"context_line":"        # we know we\u0027ll use, like info_cache and flavor."},{"line_number":673,"context_line":"        local_instances \u003d self._get_instances_on_driver(read_deleted_context)"},{"line_number":674,"context_line":"        evacuated \u003d [inst for inst in local_instances"},{"line_number":675,"context_line":"                     if inst.uuid in evacuations]"},{"line_number":676,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_5775d584","line":673,"in_reply_to":"3f79a3b5_173c3dbe","updated":"2018-10-25 16:27:09.000000000","message":"\u003e We had to backport the whack-a-mole that caused me to write the\n \u003e above patch as well, so personally I\u0027m ready to backport the\n \u003e generic fix, but I\u0027m fine with whatever the consensus is.\n\nPersonally I\u0027d prefer that we go with an isolated fix for backports, and then replace with the generic fix on master-only. Because I\u0027m a Nervous Nelly.","commit_id":"6be64ff6f4a87f74c4e321569bd1143ab394ca74"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9d6e17ad4e351a89e077437e4796742be7ce5403","unresolved":false,"context_lines":[{"line_number":670,"context_line":"        read_deleted_context \u003d context.elevated(read_deleted\u003d\u0027yes\u0027)"},{"line_number":671,"context_line":"        # TODO(mriedem): We could optimize by pre-loading the joined fields"},{"line_number":672,"context_line":"        # we know we\u0027ll use, like info_cache and flavor."},{"line_number":673,"context_line":"        local_instances \u003d self._get_instances_on_driver(read_deleted_context)"},{"line_number":674,"context_line":"        evacuated \u003d [inst for inst in local_instances"},{"line_number":675,"context_line":"                     if inst.uuid in evacuations]"},{"line_number":676,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_ebcd432f","line":673,"in_reply_to":"3f79a3b5_4de26104","updated":"2018-10-24 12:59:46.000000000","message":"I can test that out. I still want the conditional below to avoid logging errors when restarting an evacuated compute.","commit_id":"6be64ff6f4a87f74c4e321569bd1143ab394ca74"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2dd47878e9f8cecc352ee54bb2df5e360d13dd18","unresolved":false,"context_lines":[{"line_number":670,"context_line":"        read_deleted_context \u003d context.elevated(read_deleted\u003d\u0027yes\u0027)"},{"line_number":671,"context_line":"        # TODO(mriedem): We could optimize by pre-loading the joined fields"},{"line_number":672,"context_line":"        # we know we\u0027ll use, like info_cache and flavor."},{"line_number":673,"context_line":"        local_instances \u003d self._get_instances_on_driver(read_deleted_context)"},{"line_number":674,"context_line":"        evacuated \u003d [inst for inst in local_instances"},{"line_number":675,"context_line":"                     if inst.uuid in evacuations]"},{"line_number":676,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_776c91ce","line":673,"in_reply_to":"3f79a3b5_6432a110","updated":"2018-10-25 16:24:30.000000000","message":"\u003e Note that we\u0027ve had problems before where we failed to finish post-delete cleanup because we got an instance in the api, marked it as deleted, and then passed it to compute where it failed to load something trivial it needed to delete the thing. I think that\u0027s where my always-read-deleted-on-load patch came out of, after we fixed the acute situation for a backport, but it\u0027s been a while so I don\u0027t remember exactly.\n\nI remember Ide6cc5bb1fce2c9aea9fa3efdf940e8308cd9ed0 so I figured it was maybe that, but that\u0027s closer to what I\u0027m trying to fix here than what you described with the API.","commit_id":"6be64ff6f4a87f74c4e321569bd1143ab394ca74"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"11c066bfdc14016d1508da6529ca2a8cbba6ca81","unresolved":false,"context_lines":[{"line_number":670,"context_line":"        read_deleted_context \u003d context.elevated(read_deleted\u003d\u0027yes\u0027)"},{"line_number":671,"context_line":"        # TODO(mriedem): We could optimize by pre-loading the joined fields"},{"line_number":672,"context_line":"        # we know we\u0027ll use, like info_cache and flavor."},{"line_number":673,"context_line":"        local_instances \u003d self._get_instances_on_driver(read_deleted_context)"},{"line_number":674,"context_line":"        evacuated \u003d [inst for inst in local_instances"},{"line_number":675,"context_line":"                     if inst.uuid in evacuations]"},{"line_number":676,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_04fc0d0b","line":673,"in_reply_to":"3f79a3b5_6432a110","updated":"2018-10-25 16:20:20.000000000","message":"FWIW I have rebased https://review.openstack.org/#/c/575190/ on top of the regression recreate test and it does solve the issue.","commit_id":"6be64ff6f4a87f74c4e321569bd1143ab394ca74"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c1b257b317749501edc593dacfcfe656981a96c6","unresolved":false,"context_lines":[{"line_number":670,"context_line":"        read_deleted_context \u003d context.elevated(read_deleted\u003d\u0027yes\u0027)"},{"line_number":671,"context_line":"        # TODO(mriedem): We could optimize by pre-loading the joined fields"},{"line_number":672,"context_line":"        # we know we\u0027ll use, like info_cache and flavor."},{"line_number":673,"context_line":"        local_instances \u003d self._get_instances_on_driver(read_deleted_context)"},{"line_number":674,"context_line":"        evacuated \u003d [inst for inst in local_instances"},{"line_number":675,"context_line":"                     if inst.uuid in evacuations]"},{"line_number":676,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_6432a110","line":673,"in_reply_to":"3f79a3b5_649621f1","updated":"2018-10-25 16:18:13.000000000","message":"The only thing my change does differently than yours here is make it consistent and part of the behavior of the object. You\u0027re using a read_deleted\u003dTrue context here, which means you\u0027ll get the same data from it that you would if the loader does it for you. What sucks about the whack-a-mole game is that some places  attrs can be loaded regardless, and other places they can\u0027t. You\u0027re doing this at a high enough level that code below that was triggering the failed load has lost sight of the fact that it\u0027s okay to assume any attribute can be loaded. IMHO, it\u0027d be better to just make that behavior consistent everywhere.\n\nNote that we\u0027ve had problems before where we failed to finish post-delete cleanup because we got an instance in the api, marked it as deleted, and then passed it to compute where it failed to load something trivial it needed to delete the thing. I think that\u0027s where my always-read-deleted-on-load patch came out of, after we fixed the acute situation for a backport, but it\u0027s been a while so I don\u0027t remember exactly.","commit_id":"6be64ff6f4a87f74c4e321569bd1143ab394ca74"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"11c066bfdc14016d1508da6529ca2a8cbba6ca81","unresolved":false,"context_lines":[{"line_number":670,"context_line":"        read_deleted_context \u003d context.elevated(read_deleted\u003d\u0027yes\u0027)"},{"line_number":671,"context_line":"        # TODO(mriedem): We could optimize by pre-loading the joined fields"},{"line_number":672,"context_line":"        # we know we\u0027ll use, like info_cache and flavor."},{"line_number":673,"context_line":"        local_instances \u003d self._get_instances_on_driver(read_deleted_context)"},{"line_number":674,"context_line":"        evacuated \u003d [inst for inst in local_instances"},{"line_number":675,"context_line":"                     if inst.uuid in evacuations]"},{"line_number":676,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_e450f12f","line":673,"in_reply_to":"3f79a3b5_649621f1","updated":"2018-10-25 16:20:20.000000000","message":"The other thing is this bug goes back to pike, so are we going to be comfortable backporting that more generic solution given the concerns around it already with loading deleted associated records like services? I\u0027m not really comfortable with backporting that.\n\nWhat if we went with the isolate fix here which is backportable, and then in a follow up replace that with the more generic solution if we\u0027re happy with that on master?","commit_id":"6be64ff6f4a87f74c4e321569bd1143ab394ca74"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"00050aa0c3605eed5d2ca0412b2f182f0d29f57b","unresolved":false,"context_lines":[{"line_number":670,"context_line":"        read_deleted_context \u003d context.elevated(read_deleted\u003d\u0027yes\u0027)"},{"line_number":671,"context_line":"        # TODO(mriedem): We could optimize by pre-loading the joined fields"},{"line_number":672,"context_line":"        # we know we\u0027ll use, like info_cache and flavor."},{"line_number":673,"context_line":"        local_instances \u003d self._get_instances_on_driver(read_deleted_context)"},{"line_number":674,"context_line":"        evacuated \u003d [inst for inst in local_instances"},{"line_number":675,"context_line":"                     if inst.uuid in evacuations]"},{"line_number":676,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_4de26104","line":673,"in_reply_to":"3f79a3b5_ad53f514","updated":"2018-10-23 17:09:54.000000000","message":"oo, can we rebase that on top of [1] and see if it fixes the bug?\n\n[1] https://review.openstack.org/#/c/606106/","commit_id":"6be64ff6f4a87f74c4e321569bd1143ab394ca74"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"633306bf86340bcf47edac506df232c4a1bedb60","unresolved":false,"context_lines":[{"line_number":670,"context_line":"        read_deleted_context \u003d context.elevated(read_deleted\u003d\u0027yes\u0027)"},{"line_number":671,"context_line":"        # TODO(mriedem): We could optimize by pre-loading the joined fields"},{"line_number":672,"context_line":"        # we know we\u0027ll use, like info_cache and flavor."},{"line_number":673,"context_line":"        local_instances \u003d self._get_instances_on_driver(read_deleted_context)"},{"line_number":674,"context_line":"        evacuated \u003d [inst for inst in local_instances"},{"line_number":675,"context_line":"                     if inst.uuid in evacuations]"},{"line_number":676,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_173c3dbe","line":673,"in_reply_to":"3f79a3b5_e450f12f","updated":"2018-10-25 16:23:16.000000000","message":"We had to backport the whack-a-mole that caused me to write the above patch as well, so personally I\u0027m ready to backport the generic fix, but I\u0027m fine with whatever the consensus is.","commit_id":"6be64ff6f4a87f74c4e321569bd1143ab394ca74"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"46cb1eb1a1d9b4286223cd260c7a6419383e120c","unresolved":false,"context_lines":[{"line_number":670,"context_line":"        read_deleted_context \u003d context.elevated(read_deleted\u003d\u0027yes\u0027)"},{"line_number":671,"context_line":"        # TODO(mriedem): We could optimize by pre-loading the joined fields"},{"line_number":672,"context_line":"        # we know we\u0027ll use, like info_cache and flavor."},{"line_number":673,"context_line":"        local_instances \u003d self._get_instances_on_driver(read_deleted_context)"},{"line_number":674,"context_line":"        evacuated \u003d [inst for inst in local_instances"},{"line_number":675,"context_line":"                     if inst.uuid in evacuations]"},{"line_number":676,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_649621f1","line":673,"in_reply_to":"3f79a3b5_ebcd432f","updated":"2018-10-25 16:11:42.000000000","message":"\u003e This is a whack-a-mole game, which I think we shouldn\u0027t play.\n\nThe whack-a-mole game isn\u0027t fun, that\u0027s true, but as noted in https://review.openstack.org/#/c/575190/2/nova/objects/instance.py@985 there might be unintended side effects of always loading deleted records associated with the instance, like in bug 1778305 and bug 1764556. I guess if we can determine those are not issues, or would not be made worse by generically loading deleted associated records, then it\u0027s not really an immediate issue.","commit_id":"6be64ff6f4a87f74c4e321569bd1143ab394ca74"}],"nova/tests/unit/compute/test_compute_mgr.py":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"b307351a3faf606ff7eb417134aeef613014a793","unresolved":false,"context_lines":[{"line_number":3794,"context_line":"        instance_2.host \u003d \u0027not-\u0027 + our_host"},{"line_number":3795,"context_line":"        instance_2.user_id \u003d uuids.user_id"},{"line_number":3796,"context_line":"        instance_2.project_id \u003d uuids.project_id"},{"line_number":3797,"context_line":"        instance_2.deleted \u003d False"},{"line_number":3798,"context_line":""},{"line_number":3799,"context_line":"        # Only instance 2 has a migration record"},{"line_number":3800,"context_line":"        migration \u003d objects.Migration(instance_uuid\u003dinstance_2.uuid)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_b9a9f38f","line":3797,"updated":"2018-09-28 16:14:57.000000000","message":"I assume this and the couple below are just avoiding a lazy load? Might want to also set it on instance_1 for consistency.","commit_id":"1838fcde1535aae34b0647bb8b03208d58b3aa36"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4e61d93c0e40920cc2c13f3d74be19b893a149b6","unresolved":false,"context_lines":[{"line_number":3794,"context_line":"        instance_2.host \u003d \u0027not-\u0027 + our_host"},{"line_number":3795,"context_line":"        instance_2.user_id \u003d uuids.user_id"},{"line_number":3796,"context_line":"        instance_2.project_id \u003d uuids.project_id"},{"line_number":3797,"context_line":"        instance_2.deleted \u003d False"},{"line_number":3798,"context_line":""},{"line_number":3799,"context_line":"        # Only instance 2 has a migration record"},{"line_number":3800,"context_line":"        migration \u003d objects.Migration(instance_uuid\u003dinstance_2.uuid)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_eeed608e","line":3797,"in_reply_to":"3f79a3b5_b9a9f38f","updated":"2018-09-28 18:04:02.000000000","message":"Correct. I can update instance_1 if I have to respin.","commit_id":"1838fcde1535aae34b0647bb8b03208d58b3aa36"}]}