)]}'
{"nova/conductor/manager.py":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"33de8c76865f66f42faf384b55522ea2f672678e","unresolved":false,"context_lines":[{"line_number":755,"context_line":"                self._cleanup_when_reschedule_fails("},{"line_number":756,"context_line":"                    context, instance, exc, legacy_request_spec,"},{"line_number":757,"context_line":"                    requested_networks)"},{"line_number":758,"context_line":"                continue"},{"line_number":759,"context_line":""},{"line_number":760,"context_line":"            try:"},{"line_number":761,"context_line":"                # NOTE(danms): This saves the az change above, refreshes our"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_8b3a0e96","line":758,"range":{"start_line":758,"start_character":16,"end_line":758,"end_character":24},"updated":"2019-10-01 16:34:51.000000000","message":"nit: we\u0027re technically missing a test for this bit","commit_id":"549a7f6e387dfbbf12ec6b0432bbf7e26bc712d2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d4c89213f95e7273cc4da7c8d26e7a56d655ca20","unresolved":false,"context_lines":[{"line_number":755,"context_line":"                self._cleanup_when_reschedule_fails("},{"line_number":756,"context_line":"                    context, instance, exc, legacy_request_spec,"},{"line_number":757,"context_line":"                    requested_networks)"},{"line_number":758,"context_line":"                continue"},{"line_number":759,"context_line":""},{"line_number":760,"context_line":"            try:"},{"line_number":761,"context_line":"                # NOTE(danms): This saves the az change above, refreshes our"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_6e5ec0ea","line":758,"range":{"start_line":758,"start_character":16,"end_line":758,"end_character":24},"in_reply_to":"3fa7e38b_8b3a0e96","updated":"2019-10-01 17:25:43.000000000","message":"I figured removing it would make the test explode but apparently it doesn\u0027t. I\u0027ll add an assertion to the test that we don\u0027t go any further.","commit_id":"549a7f6e387dfbbf12ec6b0432bbf7e26bc712d2"}],"nova/tests/unit/conductor/test_conductor.py":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"33de8c76865f66f42faf384b55522ea2f672678e","unresolved":false,"context_lines":[{"line_number":3179,"context_line":"        host2 \u003d host_lists[0][0]"},{"line_number":3180,"context_line":""},{"line_number":3181,"context_line":"        self.conductor.build_instances("},{"line_number":3182,"context_line":"            self.context, [instance], image, filter_props,"},{"line_number":3183,"context_line":"            mock.sentinel.admin_password, mock.sentinel.injected_files,"},{"line_number":3184,"context_line":"            requested_networks, mock.sentinel.security_groups,"},{"line_number":3185,"context_line":"            request_spec\u003drequest_spec, host_lists\u003dhost_lists)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_6b5eb2ea","line":3182,"range":{"start_line":3182,"start_character":27,"end_line":3182,"end_character":35},"updated":"2019-10-01 16:34:51.000000000","message":"Continuing my nit, if we had two instances here, we could then assert the \u0027continue\u0027 worked by making sue mock_cleanup has 2 calls.","commit_id":"549a7f6e387dfbbf12ec6b0432bbf7e26bc712d2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d4c89213f95e7273cc4da7c8d26e7a56d655ca20","unresolved":false,"context_lines":[{"line_number":3179,"context_line":"        host2 \u003d host_lists[0][0]"},{"line_number":3180,"context_line":""},{"line_number":3181,"context_line":"        self.conductor.build_instances("},{"line_number":3182,"context_line":"            self.context, [instance], image, filter_props,"},{"line_number":3183,"context_line":"            mock.sentinel.admin_password, mock.sentinel.injected_files,"},{"line_number":3184,"context_line":"            requested_networks, mock.sentinel.security_groups,"},{"line_number":3185,"context_line":"            request_spec\u003drequest_spec, host_lists\u003dhost_lists)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_2e58c8f8","line":3182,"range":{"start_line":3182,"start_character":27,"end_line":3182,"end_character":35},"in_reply_to":"3fa7e38b_6b5eb2ea","updated":"2019-10-01 17:25:43.000000000","message":"I didn\u0027t do that because technically during a reschedule you\u0027re not going to have more than one instance, and since Train the only time you can get to build_instances is during a reschedule. Cells v1 used to hit it from the API for the initial build, and there is still a ton of initial build gorp in that method, but it\u0027s not something we can hit anymore (more than one instance I mean) because cells v1 was removed in Train.","commit_id":"549a7f6e387dfbbf12ec6b0432bbf7e26bc712d2"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"faf3529f45f684ab48b230a47b0ff07ae350c2d8","unresolved":false,"context_lines":[{"line_number":3192,"context_line":"            self.context, instance,"},{"line_number":3193,"context_line":"            test.MatchType(db_exc.CantStartEngineError), test.MatchType(dict),"},{"line_number":3194,"context_line":"            requested_networks)"},{"line_number":3195,"context_line":"        # Assert that we did not continue processing the instance once we"},{"line_number":3196,"context_line":"        # handled the error."},{"line_number":3197,"context_line":"        mock_save.assert_not_called()"},{"line_number":3198,"context_line":""},{"line_number":3199,"context_line":"    def test_cleanup_allocated_networks_none_requested(self):"},{"line_number":3200,"context_line":"        # Tests that we don\u0027t deallocate networks if \u0027none\u0027 were specifically"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_a1bda903","line":3197,"range":{"start_line":3195,"start_character":9,"end_line":3197,"end_character":37},"updated":"2019-10-01 18:21:49.000000000","message":"This is obvious and easy, thanks!","commit_id":"38fb7f82abd7fffc00ebc050ee5230f1137e76d8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"b440c88f2a84fb1c33f8396ee0e63946d461502c","unresolved":false,"context_lines":[{"line_number":3192,"context_line":"            self.context, instance,"},{"line_number":3193,"context_line":"            test.MatchType(db_exc.CantStartEngineError), test.MatchType(dict),"},{"line_number":3194,"context_line":"            requested_networks)"},{"line_number":3195,"context_line":"        # Assert that we did not continue processing the instance once we"},{"line_number":3196,"context_line":"        # handled the error."},{"line_number":3197,"context_line":"        mock_save.assert_not_called()"},{"line_number":3198,"context_line":""},{"line_number":3199,"context_line":"    def test_cleanup_allocated_networks_none_requested(self):"},{"line_number":3200,"context_line":"        # Tests that we don\u0027t deallocate networks if \u0027none\u0027 were specifically"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_5f0da5e6","line":3197,"range":{"start_line":3195,"start_character":9,"end_line":3197,"end_character":37},"in_reply_to":"3fa7e38b_a1bda903","updated":"2019-10-02 11:10:24.000000000","message":"First I was confused that the goal of the change is to put the instance to ERROR state but then the test assert that the instance is not saved et all. Now I see that _cleanup_when_reschedule_fails() is mocked (and asserted to be called) that that actually mocks away the instance.save call.","commit_id":"38fb7f82abd7fffc00ebc050ee5230f1137e76d8"}]}
