)]}'
{"nova/compute/manager.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b610f6026918dab1757aeac84975dd3db75fc12d","unresolved":false,"context_lines":[{"line_number":3551,"context_line":"                                context, instance, bdms\u003dbdms)"},{"line_number":3552,"context_line":""},{"line_number":3553,"context_line":"            destroy_disks \\"},{"line_number":3554,"context_line":"                \u003d self.driver.check_instance_shared_storage_local(context,"},{"line_number":3555,"context_line":"                                                                  instance)"},{"line_number":3556,"context_line":"            self.driver.destroy(context, instance, network_info,"},{"line_number":3557,"context_line":"                                block_device_info, destroy_disks)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_1cdc3674","line":3554,"updated":"2014-12-16 20:45:21.000000000","message":"You need to call self._is_instance_storage_shared here since driver.destroy takes destroy_disks as a boolean, but driver.check_instance_shared_storage_local returns either None (if not local or shared storage) or a dict with an entry to a file path on shared storage.  That\u0027s not the destroy_disks value that driver.destroy is looking for and only works because of the truthiness of the \"if destroy_disks:\" check in the driver\u0027s destroy method path.\n\nSo use _is_instance_storage_shared to get the proper boolean.\n\nThis is always super confusing for me to review because of the name of the methods in the virt driver API...","commit_id":"4e4b4215eaecad03e7dd6e7a2c06c528c4671b71"}],"nova/tests/unit/compute/test_compute.py":[{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"f2c1ad56beb707cc2b44d0ac6c52f156e604250b","unresolved":false,"context_lines":[{"line_number":5233,"context_line":"        self._test_finish_revert_resize(power_on\u003dFalse,"},{"line_number":5234,"context_line":"                                        remove_old_vm_state\u003dTrue)"},{"line_number":5235,"context_line":""},{"line_number":5236,"context_line":"    def test_revert_resize_instance_destroy_disks(self):"},{"line_number":5237,"context_line":"        # This test asserts that _is_instance_storage_shared() is called from"},{"line_number":5238,"context_line":"        # revert_resize() and the return value is passed to driver.destroy()."},{"line_number":5239,"context_line":"        # Otherwise we could regress this."}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_0d9e1c07","line":5236,"updated":"2014-12-23 12:00:59.000000000","message":"Please use `mock` for all new test-cases instead `mox` and `stubs`","commit_id":"4195c1fdd354b8c6219b8235d8374f0e00f05862"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7109f41c625f85bc3ee734639df19ee91ddbe5cd","unresolved":false,"context_lines":[{"line_number":5233,"context_line":"        self._test_finish_revert_resize(power_on\u003dFalse,"},{"line_number":5234,"context_line":"                                        remove_old_vm_state\u003dTrue)"},{"line_number":5235,"context_line":""},{"line_number":5236,"context_line":"    def test_revert_resize_instance_destroy_disks(self):"},{"line_number":5237,"context_line":"        # This test asserts that _is_instance_storage_shared() is called from"},{"line_number":5238,"context_line":"        # revert_resize() and the return value is passed to driver.destroy()."},{"line_number":5239,"context_line":"        # Otherwise we could regress this."}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_5023ac04","line":5236,"updated":"2015-01-05 20:59:23.000000000","message":"So how angry would you be if I asked that this be moved to nova.tests.unit.compute.test_compute_mgr?  I think we\u0027d like new compute manager tests moved there since it\u0027s not using a backing database (test class extends NoDbTestCase), and that\u0027s something we want for new unit tests that don\u0027t really need the DB.","commit_id":"748cc915d057eacbf6198c192dff45735aff27b0"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"3cf1e771688b90dc87958c5608ff5d8d7a9a7755","unresolved":false,"context_lines":[{"line_number":5233,"context_line":"        self._test_finish_revert_resize(power_on\u003dFalse,"},{"line_number":5234,"context_line":"                                        remove_old_vm_state\u003dTrue)"},{"line_number":5235,"context_line":""},{"line_number":5236,"context_line":"    def test_revert_resize_instance_destroy_disks(self):"},{"line_number":5237,"context_line":"        # This test asserts that _is_instance_storage_shared() is called from"},{"line_number":5238,"context_line":"        # revert_resize() and the return value is passed to driver.destroy()."},{"line_number":5239,"context_line":"        # Otherwise we could regress this."}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_d04f9c90","line":5236,"in_reply_to":"3a961159_5023ac04","updated":"2015-01-05 21:01:39.000000000","message":"Not at all, I\u0027m learning many things.  Thanks for the prompt reply, I\u0027ll get these changes posted soon.","commit_id":"748cc915d057eacbf6198c192dff45735aff27b0"},{"author":{"_account_id":14428,"name":"Bharat Kumar Kobagana","email":"bharath.kobagana@gmail.com","username":"bharatkk"},"change_message_id":"d4394a1ae5fc016ea0c20c56fcac93812b5511cf","unresolved":false,"context_lines":[{"line_number":5238,"context_line":"        # revert_resize() and the return value is passed to driver.destroy()."},{"line_number":5239,"context_line":"        # Otherwise we could regress this."},{"line_number":5240,"context_line":""},{"line_number":5241,"context_line":"        with contextlib.nested("},{"line_number":5242,"context_line":"            mock.patch.object(objects.Migration,"},{"line_number":5243,"context_line":"                              \u0027save\u0027),"},{"line_number":5244,"context_line":"            mock.patch.object(self.compute,"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_98f9a8ca","line":5241,"updated":"2015-01-06 10:14:58.000000000","message":"contextlib.nested is deprecated since 2.7: https://docs.python.org/2.7/library/contextlib.html\n\nPlease use \u0027with\u0027 instead of contextlib.nexted.","commit_id":"748cc915d057eacbf6198c192dff45735aff27b0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bf0340cd522113669816adc751e8b747b040cc96","unresolved":false,"context_lines":[{"line_number":5263,"context_line":"            migrate_instance_start,"},{"line_number":5264,"context_line":"        ):"},{"line_number":5265,"context_line":""},{"line_number":5266,"context_line":"            c \u003d context.get_admin_context()"},{"line_number":5267,"context_line":"            instance \u003d self._create_fake_instance_obj()"},{"line_number":5268,"context_line":"            migration \u003d objects.Migration()"},{"line_number":5269,"context_line":"            migration.source_compute \u003d instance[\u0027host\u0027]"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_f069f8f8","line":5266,"updated":"2015-01-05 21:01:10.000000000","message":"Unless you actually need an admin context, just use self.context.","commit_id":"748cc915d057eacbf6198c192dff45735aff27b0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7109f41c625f85bc3ee734639df19ee91ddbe5cd","unresolved":false,"context_lines":[{"line_number":5271,"context_line":"            # inform compute that this instance uses shared storage"},{"line_number":5272,"context_line":"            _is_instance_storage_shared.return_value \u003d True"},{"line_number":5273,"context_line":""},{"line_number":5274,"context_line":"            def fake_driver_destroy(context, instance, network_info,"},{"line_number":5275,"context_line":"                                    block_device_info, destroy_disks):"},{"line_number":5276,"context_line":"                # since shared storage is used, we should not be instructed to"},{"line_number":5277,"context_line":"                # destroy disks here"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_f0e498a9","line":5274,"updated":"2015-01-05 20:59:23.000000000","message":"Don\u0027t need this if you use driver_destroy.assert_called_once_with.","commit_id":"748cc915d057eacbf6198c192dff45735aff27b0"},{"author":{"_account_id":14428,"name":"Bharat Kumar Kobagana","email":"bharath.kobagana@gmail.com","username":"bharatkk"},"change_message_id":"d4394a1ae5fc016ea0c20c56fcac93812b5511cf","unresolved":false,"context_lines":[{"line_number":5275,"context_line":"                                    block_device_info, destroy_disks):"},{"line_number":5276,"context_line":"                # since shared storage is used, we should not be instructed to"},{"line_number":5277,"context_line":"                # destroy disks here"},{"line_number":5278,"context_line":"                self.assertEqual(destroy_disks, False)"},{"line_number":5279,"context_line":"            driver_destroy.side_effect \u003d fake_driver_destroy"},{"line_number":5280,"context_line":""},{"line_number":5281,"context_line":"            self.compute.revert_resize(context\u003dc,"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_38d99457","line":5278,"updated":"2015-01-06 10:14:58.000000000","message":"Looks like the order of the parameters are reversed. It should be expected, actual. self.assertEqual(false, destroy_disks)","commit_id":"748cc915d057eacbf6198c192dff45735aff27b0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bf2ca8d98da8897def746ba72d26fca6ed8910c9","unresolved":false,"context_lines":[{"line_number":5275,"context_line":"                                    block_device_info, destroy_disks):"},{"line_number":5276,"context_line":"                # since shared storage is used, we should not be instructed to"},{"line_number":5277,"context_line":"                # destroy disks here"},{"line_number":5278,"context_line":"                self.assertEqual(destroy_disks, False)"},{"line_number":5279,"context_line":"            driver_destroy.side_effect \u003d fake_driver_destroy"},{"line_number":5280,"context_line":""},{"line_number":5281,"context_line":"            self.compute.revert_resize(context\u003dc,"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_891cf567","line":5278,"in_reply_to":"3a961159_38d99457","updated":"2015-01-06 16:33:53.000000000","message":"Or just use self.assertFalse(destroy_disks), but this whole block can be removed as noted above.","commit_id":"748cc915d057eacbf6198c192dff45735aff27b0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7109f41c625f85bc3ee734639df19ee91ddbe5cd","unresolved":false,"context_lines":[{"line_number":5283,"context_line":"                                       instance\u003dinstance,"},{"line_number":5284,"context_line":"                                       reservations\u003dNone)"},{"line_number":5285,"context_line":""},{"line_number":5286,"context_line":"            self.assertTrue(_is_instance_storage_shared.called)"},{"line_number":5287,"context_line":"            self.assertTrue(driver_destroy.called)"},{"line_number":5288,"context_line":""},{"line_number":5289,"context_line":"    def _test_cleanup_stored_instance_types(self, old, new, revert\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_70734816","line":5286,"updated":"2015-01-05 20:59:23.000000000","message":"nit: use _is_instance_storage_shared.assert_called_once_with() instead.","commit_id":"748cc915d057eacbf6198c192dff45735aff27b0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7109f41c625f85bc3ee734639df19ee91ddbe5cd","unresolved":false,"context_lines":[{"line_number":5284,"context_line":"                                       reservations\u003dNone)"},{"line_number":5285,"context_line":""},{"line_number":5286,"context_line":"            self.assertTrue(_is_instance_storage_shared.called)"},{"line_number":5287,"context_line":"            self.assertTrue(driver_destroy.called)"},{"line_number":5288,"context_line":""},{"line_number":5289,"context_line":"    def _test_cleanup_stored_instance_types(self, old, new, revert\u003dFalse):"},{"line_number":5290,"context_line":"        instance \u003d self._create_fake_instance_obj()"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_10d3e4d5","line":5287,"updated":"2015-01-05 20:59:23.000000000","message":"Same here, use assert_called_once_with for mock calls that are only called once, else use assert_called_with from mocked objects if it\u0027s called more than once with the same args.","commit_id":"748cc915d057eacbf6198c192dff45735aff27b0"}],"nova/tests/unit/compute/test_compute_mgr.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"262a07043e5feb1cbddeed06ae10b580017792a0","unresolved":false,"context_lines":[{"line_number":3252,"context_line":"            self.assertEqual(\"error\", self.migration.status)"},{"line_number":3253,"context_line":"            migration_save.assert_has_calls([mock.call(elevated_context)])"},{"line_number":3254,"context_line":""},{"line_number":3255,"context_line":"    @mock.patch.object(objects.InstanceActionEvent, \u0027event_start\u0027)"},{"line_number":3256,"context_line":"    @mock.patch.object(objects.InstanceActionEvent,"},{"line_number":3257,"context_line":"                       \u0027event_finish_with_failure\u0027)"},{"line_number":3258,"context_line":"    def test_revert_resize_instance_destroy_disks(self,"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_e2dcc01a","line":3255,"updated":"2015-01-14 22:56:24.000000000","message":"No longer needed after rebasing on top of my change.","commit_id":"bcca6809016cf71e5b2c1e8eec9434ed1b6e2d42"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"262a07043e5feb1cbddeed06ae10b580017792a0","unresolved":false,"context_lines":[{"line_number":3273,"context_line":"        @mock.patch.object(self.compute.network_api, \u0027migrate_instance_start\u0027)"},{"line_number":3274,"context_line":"        @mock.patch.object(self.compute.conductor_api, \u0027notify_usage_exists\u0027)"},{"line_number":3275,"context_line":"        @mock.patch.object(self.migration, \u0027save\u0027)"},{"line_number":3276,"context_line":"        @mock.patch.object(db, \u0027instance_fault_create\u0027)"},{"line_number":3277,"context_line":"        @mock.patch.object(compute_utils, \u0027add_instance_fault_from_exc\u0027)"},{"line_number":3278,"context_line":"        @mock.patch.object(objects.BlockDeviceMappingList,"},{"line_number":3279,"context_line":"                           \u0027get_by_instance_uuid\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_42e834b3","line":3276,"updated":"2015-01-14 22:56:24.000000000","message":"not needed","commit_id":"bcca6809016cf71e5b2c1e8eec9434ed1b6e2d42"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"262a07043e5feb1cbddeed06ae10b580017792a0","unresolved":false,"context_lines":[{"line_number":3274,"context_line":"        @mock.patch.object(self.compute.conductor_api, \u0027notify_usage_exists\u0027)"},{"line_number":3275,"context_line":"        @mock.patch.object(self.migration, \u0027save\u0027)"},{"line_number":3276,"context_line":"        @mock.patch.object(db, \u0027instance_fault_create\u0027)"},{"line_number":3277,"context_line":"        @mock.patch.object(compute_utils, \u0027add_instance_fault_from_exc\u0027)"},{"line_number":3278,"context_line":"        @mock.patch.object(objects.BlockDeviceMappingList,"},{"line_number":3279,"context_line":"                           \u0027get_by_instance_uuid\u0027)"},{"line_number":3280,"context_line":"        def do_test(get_by_instance_uuid,"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_62d0d0f7","line":3277,"updated":"2015-01-14 22:56:24.000000000","message":"not needed","commit_id":"bcca6809016cf71e5b2c1e8eec9434ed1b6e2d42"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"262a07043e5feb1cbddeed06ae10b580017792a0","unresolved":false,"context_lines":[{"line_number":3306,"context_line":""},{"line_number":3307,"context_line":"            # since shared storage is used, we should not be instructed to"},{"line_number":3308,"context_line":"            # destroy disks here"},{"line_number":3309,"context_line":"            destroy.assert_called_once_with(mock.ANY, mock.ANY, mock.ANY,"},{"line_number":3310,"context_line":"                                            mock.ANY, False)"},{"line_number":3311,"context_line":""},{"line_number":3312,"context_line":"        do_test()"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_c2d344ff","line":3309,"updated":"2015-01-14 22:56:24.000000000","message":"we should pass in the context and instance that we have in scope, since we mock out the calls to get the nw info and bdm we can use mock.ANY for those.","commit_id":"bcca6809016cf71e5b2c1e8eec9434ed1b6e2d42"}]}