From 8153cddad194ffeb3d21d59df83f8f20ddf89180 Mon Sep 17 00:00:00 2001 From: Jay Pipes Date: Wed, 24 Oct 2018 10:16:26 -0400 Subject: [PATCH] quota: remove QuotaDriver.destroy_all_by_project() Removes the unnecessary QuotaDriver.destroy_all_by_project() and destroy_all_by_project_and_users() methods. nova.objects.Quotas already had these methods and this patch changes the one place that was calling the old QuotaDriver method (from the nova API /quota_sets endpoint) to just call the nova.objects.Quotas methods of the same name. Note that the NoopQuotaDriver's destroy_all_by_project() and destroy_all_by_project_and_user() methods were no-ops. Now the quota-sets API will be calling the objects.Quotas.destroy_xxx() methods which will raise ProjectUserQuotaNotFound instead of returning a 204. If the user is calling DELETE /os-quota-sets and there is the Noop quota driver configured, and the response is a 404 Not Found, do we really care? In the future, we should be getting rid of the os-quota-sets API entirely and using Keystone's /limits API. One more set of methods gone from QuotaDriver... Change-Id: Ifc0a409bd179807db18f2e7b59ea9d4d67e9a798 --- nova/api/openstack/compute/quota_sets.py | 6 +- nova/quota.py | 59 ------------------- .../unit/api/openstack/compute/test_quotas.py | 4 +- nova/tests/unit/test_quota.py | 29 --------- 4 files changed, 5 insertions(+), 93 deletions(-) diff --git a/nova/api/openstack/compute/quota_sets.py b/nova/api/openstack/compute/quota_sets.py index e460d93f9b9a..1112bf997a9f 100644 --- a/nova/api/openstack/compute/quota_sets.py +++ b/nova/api/openstack/compute/quota_sets.py @@ -275,7 +275,7 @@ class QuotaSetsController(wsgi.Controller): params = urlparse.parse_qs(req.environ.get('QUERY_STRING', '')) user_id = params.get('user_id', [None])[0] if user_id: - QUOTAS.destroy_all_by_project_and_user(context, - id, user_id) + objects.Quotas.destroy_all_by_project_and_user( + context, id, user_id) else: - QUOTAS.destroy_all_by_project(context, id) + objects.Quotas.destroy_all_by_project(context, id) diff --git a/nova/quota.py b/nova/quota.py index 8e520056d306..f5ba76c6dace 100644 --- a/nova/quota.py +++ b/nova/quota.py @@ -612,26 +612,6 @@ class DbQuotaDriver(object): quotas=quotas_exceeded, usages={}, headroom=headroom) - def destroy_all_by_project_and_user(self, context, project_id, user_id): - """Destroy all quotas associated with a project and user. - - :param context: The request context, for access checks. - :param project_id: The ID of the project being deleted. - :param user_id: The ID of the user being deleted. - """ - - objects.Quotas.destroy_all_by_project_and_user(context, project_id, - user_id) - - def destroy_all_by_project(self, context, project_id): - """Destroy all quotas associated with a project. - - :param context: The request context, for access checks. - :param project_id: The ID of the project being deleted. - """ - - objects.Quotas.destroy_all_by_project(context, project_id) - class NoopQuotaDriver(object): """Driver that turns quotas calls into no-ops and pretends that quotas @@ -805,23 +785,6 @@ class NoopQuotaDriver(object): """ pass - def destroy_all_by_project_and_user(self, context, project_id, user_id): - """Destroy all quotas associated with a project and user. - - :param context: The request context, for access checks. - :param project_id: The ID of the project being deleted. - :param user_id: The ID of the user being deleted. - """ - pass - - def destroy_all_by_project(self, context, project_id): - """Destroy all quotas associated with a project. - - :param context: The request context, for access checks. - :param project_id: The ID of the project being deleted. - """ - pass - class BaseResource(object): """Describe a single resource for quota checking.""" @@ -1116,28 +1079,6 @@ class QuotaEngine(object): context, self._resources, project_values=project_values, user_values=user_values, project_id=project_id, user_id=user_id) - def destroy_all_by_project_and_user(self, context, project_id, user_id): - """Destroy all quotas, usages, and reservations associated with a - project and user. - - :param context: The request context, for access checks. - :param project_id: The ID of the project being deleted. - :param user_id: The ID of the user being deleted. - """ - - self._driver.destroy_all_by_project_and_user(context, - project_id, user_id) - - def destroy_all_by_project(self, context, project_id): - """Destroy all quotas, usages, and reservations associated with a - project. - - :param context: The request context, for access checks. - :param project_id: The ID of the project being deleted. - """ - - self._driver.destroy_all_by_project(context, project_id) - @property def resources(self): return sorted(self._resources.keys()) diff --git a/nova/tests/unit/api/openstack/compute/test_quotas.py b/nova/tests/unit/api/openstack/compute/test_quotas.py index bf481f164ad3..c8d6e86c8e53 100644 --- a/nova/tests/unit/api/openstack/compute/test_quotas.py +++ b/nova/tests/unit/api/openstack/compute/test_quotas.py @@ -275,7 +275,7 @@ class QuotaSetsTestV21(BaseQuotaSetsTest): body = {'quota_set': self.default_quotas} self._quotas_update_bad_request_case(body) - @mock.patch.object(quota.QUOTAS, 'destroy_all_by_project') + @mock.patch('nova.objects.Quotas.destroy_all_by_project') def test_quotas_delete(self, mock_destroy_all_by_project): req = self._get_http_request() res = self.controller.delete(req, 1234) @@ -470,7 +470,7 @@ class UserQuotasTestV21(BaseQuotaSetsTest): self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, req, 'update_me', body=body) - @mock.patch.object(quota.QUOTAS, "destroy_all_by_project_and_user") + @mock.patch('nova.objects.Quotas.destroy_all_by_project_and_user') def test_user_quotas_delete(self, mock_destroy_all_by_project_and_user): url = '/v2/fake4/os-quota-sets/1234?user_id=1' req = self._get_http_request(url) diff --git a/nova/tests/unit/test_quota.py b/nova/tests/unit/test_quota.py index 4ad3d572bafe..1221a66ef7f5 100644 --- a/nova/tests/unit/test_quota.py +++ b/nova/tests/unit/test_quota.py @@ -317,13 +317,6 @@ class FakeDriver(object): self.called.append(('limit_check_project_and_user', context, resources, project_values, user_values, project_id, user_id)) - def destroy_all_by_project_and_user(self, context, project_id, user_id): - self.called.append(('destroy_all_by_project_and_user', context, - project_id, user_id)) - - def destroy_all_by_project(self, context, project_id): - self.called.append(('destroy_all_by_project', context, project_id)) - class BaseResourceTestCase(test.TestCase): def test_no_flag(self): @@ -567,28 +560,6 @@ class QuotaEngineTestCase(test.TestCase): None, None)], driver.called) - def test_destroy_all_by_project_and_user(self): - context = FakeContext(None, None) - driver = FakeDriver() - quota_obj = self._make_quota_obj(driver) - quota_obj.destroy_all_by_project_and_user(context, - 'test_project', 'fake_user') - - self.assertEqual(driver.called, [ - ('destroy_all_by_project_and_user', context, 'test_project', - 'fake_user'), - ]) - - def test_destroy_all_by_project(self): - context = FakeContext(None, None) - driver = FakeDriver() - quota_obj = self._make_quota_obj(driver) - quota_obj.destroy_all_by_project(context, 'test_project') - - self.assertEqual(driver.called, [ - ('destroy_all_by_project', context, 'test_project'), - ]) - def test_resources(self): quota_obj = self._make_quota_obj(None)