)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"d67233dfb726f58e8395356e4bd20715979c25a5","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If there\u0027s a scheduler failure the instance is put into an error status"},{"line_number":10,"context_line":"and that\u0027s what should be returned by an instance show/list. However"},{"line_number":11,"context_line":"because the BuildRequest is not deleted that\u0027s what actually gets"},{"line_number":12,"context_line":"returned. Delete the BuildRequest on scheduler failures."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: Ia6c87aecc1484a2366d50822b669291c9f954545"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"fa7ab95a_b9ce2181","line":11,"updated":"2016-09-01 14:20:31.000000000","message":"In case you want to be super clear, I\u0027m struggling to understand what the \"that\" in \"that\u0027s what\" is referring to in both of those cases. I guess you mean that the instance should be returned in both cases, never the build request.","commit_id":"f577f650c7ca9d8dd66eaec919e4805c09d16f6d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"970c37bd5e747db0ac931efcd434f86d6b12afb7","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If there\u0027s a scheduler failure the instance is put into an error status"},{"line_number":10,"context_line":"and that\u0027s what should be returned by an instance show/list. However"},{"line_number":11,"context_line":"because the BuildRequest is not deleted that\u0027s what actually gets"},{"line_number":12,"context_line":"returned. Delete the BuildRequest on scheduler failures."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: Ia6c87aecc1484a2366d50822b669291c9f954545"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"fa7ab95a_2b60f434","line":11,"in_reply_to":"fa7ab95a_b9ce2181","updated":"2016-09-01 14:32:55.000000000","message":"I think the confusion in some of this series (I had some of this yesterday), was that this is all building up to the final change in the series that we want which is:\n\nhttps://review.openstack.org/#/c/356138/\n\nSo some of the comments and commit messages are written as things that will happen with https://review.openstack.org/#/c/356138/ but don\u0027t happen as of this change. Laski and I had talked about squashing some of these changes into https://review.openstack.org/#/c/356138/ but there is a tradeoff with how complicated we want to make https://review.openstack.org/#/c/356138/ and it\u0027s already pretty complicated, so this is making it piecemeal.","commit_id":"f577f650c7ca9d8dd66eaec919e4805c09d16f6d"}],"nova/conductor/manager.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"0eae1e38228dfeed1d7889914c03c0e9dbdb7b3e","unresolved":false,"context_lines":[{"line_number":490,"context_line":"                    exc, request_spec)"},{"line_number":491,"context_line":"                try:"},{"line_number":492,"context_line":"                    # If the BuildRequest stays around then instance show/lists"},{"line_number":493,"context_line":"                    # will pull from it rather than the errored instance."},{"line_number":494,"context_line":"                    self._destroy_build_request(context, instance)"},{"line_number":495,"context_line":"                except exception.BuildRequestNotFound:"},{"line_number":496,"context_line":"                    pass"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa7ab95a_9e29433a","line":493,"updated":"2016-09-01 14:04:22.000000000","message":"This only happens if we get through build_request_spec() and then fail at schedule_instances(), is that right?","commit_id":"39819da18b7810758d23eb4ba3441c108f02e298"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"d7dcce755effa3b6b00de635ed6a73dad2828111","unresolved":false,"context_lines":[{"line_number":490,"context_line":"                    exc, request_spec)"},{"line_number":491,"context_line":"                try:"},{"line_number":492,"context_line":"                    # If the BuildRequest stays around then instance show/lists"},{"line_number":493,"context_line":"                    # will pull from it rather than the errored instance."},{"line_number":494,"context_line":"                    self._destroy_build_request(context, instance)"},{"line_number":495,"context_line":"                except exception.BuildRequestNotFound:"},{"line_number":496,"context_line":"                    pass"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa7ab95a_fe932ff8","line":493,"in_reply_to":"fa7ab95a_9e29433a","updated":"2016-09-01 14:10:53.000000000","message":"Right. After scheduling there should no longer be a BuildRequest, but the failure case wasn\u0027t keeping that invariant.","commit_id":"39819da18b7810758d23eb4ba3441c108f02e298"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"b7d703fbfd366c2263040fe3beed2c7feaa83eb6","unresolved":false,"context_lines":[{"line_number":493,"context_line":"                    # will pull from it rather than the errored instance."},{"line_number":494,"context_line":"                    self._destroy_build_request(context, instance)"},{"line_number":495,"context_line":"                except exception.BuildRequestNotFound:"},{"line_number":496,"context_line":"                    pass"},{"line_number":497,"context_line":"                self._cleanup_allocated_networks("},{"line_number":498,"context_line":"                    context, instance, requested_networks)"},{"line_number":499,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa7ab95a_1ee37334","line":496,"updated":"2016-09-01 14:04:12.000000000","message":"I did a double take on this line, I keep wondering about just a debug log or something. But we do so much of that no one reads it anyway, so maybe leave this as it is.","commit_id":"39819da18b7810758d23eb4ba3441c108f02e298"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"d7dcce755effa3b6b00de635ed6a73dad2828111","unresolved":false,"context_lines":[{"line_number":493,"context_line":"                    # will pull from it rather than the errored instance."},{"line_number":494,"context_line":"                    self._destroy_build_request(context, instance)"},{"line_number":495,"context_line":"                except exception.BuildRequestNotFound:"},{"line_number":496,"context_line":"                    pass"},{"line_number":497,"context_line":"                self._cleanup_allocated_networks("},{"line_number":498,"context_line":"                    context, instance, requested_networks)"},{"line_number":499,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa7ab95a_be77e7d2","line":496,"in_reply_to":"fa7ab95a_1ee37334","updated":"2016-09-01 14:10:53.000000000","message":"I\u0027m not sure it\u0027s useful here. We just want the BuildRequest removed, it doesn\u0027t really matter if it existed before or not.","commit_id":"39819da18b7810758d23eb4ba3441c108f02e298"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"781e795c5923d9f6b4bb5efa08c2e6d93f88b478","unresolved":false,"context_lines":[{"line_number":476,"context_line":"            # check retry policy. Rather ugly use of instances[0]..."},{"line_number":477,"context_line":"            # but if we\u0027ve exceeded max retries... then we really only"},{"line_number":478,"context_line":"            # have a single instance."},{"line_number":479,"context_line":"            scheduler_utils.populate_retry("},{"line_number":480,"context_line":"                filter_properties, instances[0].uuid)"},{"line_number":481,"context_line":"            request_spec \u003d scheduler_utils.build_request_spec("},{"line_number":482,"context_line":"                    context, image, instances)"}],"source_content_type":"text/x-python","patch_set":2,"id":"df87a7cf_087bd47a","line":479,"range":{"start_line":479,"start_character":28,"end_line":479,"end_character":42},"updated":"2017-12-19 01:55:41.000000000","message":"bug 1736946 - oopsy doodle!","commit_id":"f577f650c7ca9d8dd66eaec919e4805c09d16f6d"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"efd9c849ed476b3fccb72d518a667c0932da8fd9","unresolved":false,"context_lines":[{"line_number":488,"context_line":"                self._set_vm_state_and_notify("},{"line_number":489,"context_line":"                    context, instance.uuid, \u0027build_instances\u0027, updates,"},{"line_number":490,"context_line":"                    exc, request_spec)"},{"line_number":491,"context_line":"                try:"},{"line_number":492,"context_line":"                    # If the BuildRequest stays around then instance show/lists"},{"line_number":493,"context_line":"                    # will pull from it rather than the errored instance."},{"line_number":494,"context_line":"                    self._destroy_build_request(context, instance)"},{"line_number":495,"context_line":"                except exception.BuildRequestNotFound:"},{"line_number":496,"context_line":"                    pass"},{"line_number":497,"context_line":"                self._cleanup_allocated_networks("},{"line_number":498,"context_line":"                    context, instance, requested_networks)"},{"line_number":499,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_d9e20501","line":496,"range":{"start_line":491,"start_character":0,"end_line":496,"end_character":24},"updated":"2016-09-01 14:17:59.000000000","message":"The problem I see is that we\u0027re catching a global Exception and then return in L495 so then _destroy_build_request() is not called.\n\nI\u0027m not sure I like to see a duplicate here from L519 but given I don\u0027t find a better and simple solution (I thought of having a contextmanager around _select_destination() that would delete BuildRequest in the finally statement but that sounds a bit wedgy), I think that\u0027s probably fine.","commit_id":"f577f650c7ca9d8dd66eaec919e4805c09d16f6d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b27bd3d1711f4c037dd0b486c7414d4b7da3568b","unresolved":false,"context_lines":[{"line_number":515,"context_line":""},{"line_number":516,"context_line":"            inst_mapping \u003d self._populate_instance_mapping(context, instance,"},{"line_number":517,"context_line":"                                                           host)"},{"line_number":518,"context_line":"            try:"},{"line_number":519,"context_line":"                self._destroy_build_request(context, instance)"},{"line_number":520,"context_line":"            except exception.BuildRequestNotFound:"},{"line_number":521,"context_line":"                # This indicates an instance delete has been requested in the"},{"line_number":522,"context_line":"                # API. Stop the build, cleanup the instance_mapping and"},{"line_number":523,"context_line":"                # potentially the block_device_mappings"},{"line_number":524,"context_line":"                # TODO(alaski): Handle block_device_mapping cleanup"},{"line_number":525,"context_line":"                if inst_mapping:"},{"line_number":526,"context_line":"                    inst_mapping.destroy()"},{"line_number":527,"context_line":"                return"},{"line_number":528,"context_line":""},{"line_number":529,"context_line":"            self.compute_rpcapi.build_and_run_instance(context,"},{"line_number":530,"context_line":"                    instance\u003dinstance, host\u003dhost[\u0027host\u0027], image\u003dimage,"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_ae6892bb","line":527,"range":{"start_line":518,"start_character":12,"end_line":527,"end_character":22},"updated":"2016-09-01 14:39:57.000000000","message":"Oh yeah now I see what Sylvain was saying. I thought we\u0027d get here and have problems, but we return above on 499. This certainly does get a bit hard to follow with all of the returns in the middle of the function.\n\nAnother simple cleanup in Ocata would be to just pull some of the chunks of this method out into private helper methods, like block 486-498, pull that out into a cleanup method or something and then return.","commit_id":"f577f650c7ca9d8dd66eaec919e4805c09d16f6d"}]}