compute, volume: Improve 'update_quota_set'

The implementations of these were rather confusing and required two
separate sets of arguments. Simplify them based on the
'delete_quota_set' methods.

Change-Id: I8bb0bfb039593c5b59f1f9c16523a090d899f099
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

openstack/block_storage/v2/_proxy.py

@@ -10,6 +10,8 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+import warnings
+
 from openstack.block_storage import _base_proxy
 from openstack.block_storage.v2 import backup as _backup
 from openstack.block_storage.v2 import capabilities as _capabilities
@@ -23,6 +25,7 @@
 from openstack.block_storage.v2 import volume as _volume
 from openstack.identity.v3 import project as _project
 from openstack import resource
+from openstack import warnings as os_warnings
 
 
 class Proxy(_base_proxy.BaseBlockStorageProxy):
@@ -769,22 +772,37 @@ def revert_quota_set(self, project, **query):
             query = {}
         return res.delete(self, **query)
 
-    def update_quota_set(self, quota_set, query=None, **attrs):
+    def update_quota_set(self, project, **attrs):
         """Update a QuotaSet.
 
-        :param quota_set: Either the ID of a quota_set or a
-            :class:`~openstack.block_storage.v2.quota_set.QuotaSet` instance.
-        :param dict query: Optional parameters to be used with update call.
+        :param project: ID or instance of
+            :class:`~openstack.identity.project.Project` of the project for
+            which the quota should be reset.
         :param attrs: The attributes to update on the QuotaSet represented
             by ``quota_set``.
 
         :returns: The updated QuotaSet
-        :rtype: :class:`~openstack.block_storage.v2.quota_set.QuotaSet`
-        """
-        res = self._get_resource(_quota_set.QuotaSet, quota_set, **attrs)
-        if not query:
-            query = {}
-        return res.commit(self, **query)
+        :rtype: :class:`~openstack.block_storage.v3.quota_set.QuotaSet`
+        """
+        if 'project_id' in attrs or isinstance(project, _quota_set.QuotaSet):
+            warnings.warn(
+                "The signature of 'update_quota_set' has changed and it "
+                "now expects a Project as the first argument, in line "
+                "with the other quota set methods.",
+                os_warnings.RemovedInSDK50Warning,
+            )
+            if isinstance(project, _quota_set.QuotaSet):
+                attrs['project_id'] = project.project_id
+
+            # cinder doesn't support any query parameters so we simply pop
+            # these
+            if 'query' in attrs:
+                attrs.pop('params')
+        else:
+            project = self._get_resource(_project.Project, project)
+            attrs['project_id'] = project.id
+
+        return self._update(_quota_set.QuotaSet, None, **attrs)
 
     # ====== VOLUME METADATA ======
     def get_volume_metadata(self, volume):

openstack/block_storage/v3/_proxy.py

@@ -11,6 +11,7 @@
 # under the License.
 
 import typing as ty
+import warnings
 
 from openstack.block_storage import _base_proxy
 from openstack.block_storage.v3 import attachment as _attachment
@@ -36,6 +37,7 @@
 from openstack.identity.v3 import project as _project
 from openstack import resource
 from openstack import utils
+from openstack import warnings as os_warnings
 
 
 class Proxy(_base_proxy.BaseBlockStorageProxy):
@@ -1794,22 +1796,37 @@ def revert_quota_set(self, project, **query):
 
         return res.delete(self, **query)
 
-    def update_quota_set(self, quota_set, query=None, **attrs):
+    def update_quota_set(self, project, **attrs):
         """Update a QuotaSet.
 
-        :param quota_set: Either the ID of a quota_set or a
-            :class:`~openstack.block_storage.v3.quota_set.QuotaSet` instance.
-        :param dict query: Optional parameters to be used with update call.
+        :param project: ID or instance of
+            :class:`~openstack.identity.project.Project` of the project for
+            which the quota should be reset.
         :param attrs: The attributes to update on the QuotaSet represented
             by ``quota_set``.
 
         :returns: The updated QuotaSet
         :rtype: :class:`~openstack.block_storage.v3.quota_set.QuotaSet`
         """
-        res = self._get_resource(_quota_set.QuotaSet, quota_set, **attrs)
-        if not query:
-            query = {}
-        return res.commit(self, **query)
+        if 'project_id' in attrs or isinstance(project, _quota_set.QuotaSet):
+            warnings.warn(
+                "The signature of 'update_quota_set' has changed and it "
+                "now expects a Project as the first argument, in line "
+                "with the other quota set methods.",
+                os_warnings.RemovedInSDK50Warning,
+            )
+            if isinstance(project, _quota_set.QuotaSet):
+                attrs['project_id'] = project.project_id
+
+            # cinder doesn't support any query parameters so we simply pop
+            # these
+            if 'query' in attrs:
+                attrs.pop('params')
+        else:
+            project = self._get_resource(_project.Project, project)
+            attrs['project_id'] = project.id
+
+        return self._update(_quota_set.QuotaSet, None, **attrs)
 
     # ====== SERVICES ======
     @ty.overload

openstack/cloud/_block_storage.py

@@ -13,7 +13,6 @@
 import warnings
 
 from openstack.block_storage.v3._proxy import Proxy
-from openstack.block_storage.v3 import quota_set as _qs
 from openstack.cloud import _utils
 from openstack import exceptions
 from openstack import warnings as os_warnings
@@ -842,12 +841,9 @@ def set_volume_quotas(self, name_or_id, **kwargs):
         :raises: :class:`~openstack.exceptions.SDKException` if the resource to
             set the quota does not exist.
         """
+        project = self.identity.find_project(name_or_id, ignore_missing=False)
 
-        proj = self.identity.find_project(name_or_id, ignore_missing=False)
-
-        self.block_storage.update_quota_set(
-            _qs.QuotaSet(project_id=proj.id), **kwargs
-        )
+        self.block_storage.update_quota_set(project=project, **kwargs)
 
     def get_volume_quotas(self, name_or_id):
         """Get volume quotas for a project

openstack/cloud/_compute.py

@@ -21,7 +21,6 @@
 from openstack.cloud import exc
 from openstack.cloud import meta
 from openstack.compute.v2._proxy import Proxy
-from openstack.compute.v2 import quota_set as _qs
 from openstack.compute.v2 import server as _server
 from openstack import exceptions
 from openstack import utils
@@ -1774,11 +1773,9 @@ def set_compute_quotas(self, name_or_id, **kwargs):
         :raises: :class:`~openstack.exceptions.SDKException` if the resource to
             set the quota does not exist.
         """
-        proj = self.identity.find_project(name_or_id, ignore_missing=False)
+        project = self.identity.find_project(name_or_id, ignore_missing=False)
         kwargs['force'] = True
-        self.compute.update_quota_set(
-            _qs.QuotaSet(project_id=proj.id), **kwargs
-        )
+        self.compute.update_quota_set(project=project, **kwargs)
 
     def get_compute_quotas(self, name_or_id):
         """Get quota for a project

openstack/compute/v2/_proxy.py

@@ -37,6 +37,7 @@
 from openstack.compute.v2 import volume_attachment as _volume_attachment
 from openstack import exceptions
 from openstack.identity.v3 import project as _project
+from openstack.identity.v3 import user as _user
 from openstack.network.v2 import security_group as _sg
 from openstack import proxy
 from openstack import resource
@@ -2494,38 +2495,59 @@ def revert_quota_set(self, project, **query):
 
         :param project: ID or instance of
             :class:`~openstack.identity.project.Project` of the project for
-            which the quota should be resetted.
+            which the quota should be reset.
         :param dict query: Additional parameters to be used.
 
         :returns: ``None``
         """
         project = self._get_resource(_project.Project, project)
         res = self._get_resource(
-            _quota_set.QuotaSet,
-            None,
-            project_id=project.id,
+            _quota_set.QuotaSet, None, project_id=project.id
         )
 
         if not query:
             query = {}
         return res.delete(self, **query)
 
-    def update_quota_set(self, quota_set, query=None, **attrs):
+    def update_quota_set(self, project, *, user=None, **attrs):
         """Update a QuotaSet.
 
-        :param quota_set: Either the ID of a quota_set or a
-            :class:`~openstack.compute.v2.quota_set.QuotaSet` instance.
-        :param dict query: Optional parameters to be used with update call.
+        :param project: ID or instance of
+            :class:`~openstack.identity.project.Project` of the project for
+            which the quota should be reset.
+        :param user_id: Optional ID of the user to set quotas as.
         :param attrs: The attributes to update on the QuotaSet represented
             by ``quota_set``.
 
         :returns: The updated QuotaSet
         :rtype: :class:`~openstack.compute.v2.quota_set.QuotaSet`
         """
-        res = self._get_resource(_quota_set.QuotaSet, quota_set, **attrs)
-        if not query:
-            query = {}
-        return res.commit(self, **query)
+        query = {}
+
+        if 'project_id' in attrs or isinstance(project, _quota_set.QuotaSet):
+            warnings.warn(
+                "The signature of 'update_quota_set' has changed and it "
+                "now expects a Project as the first argument, in line "
+                "with the other quota set methods.",
+                os_warnings.RemovedInSDK50Warning,
+            )
+            if isinstance(project, _quota_set.QuotaSet):
+                attrs['project_id'] = project.project_id
+
+            if 'query' in attrs:
+                query = attrs.pop('query')
+        else:
+            project = self._get_resource(_project.Project, project)
+            attrs['project_id'] = project.id
+
+            if user:
+                user = self._get_resource(_user.User, user)
+                query['user_id'] = user.id
+
+        # we don't use Proxy._update since that doesn't allow passing arbitrary
+        # query string parameters
+        quota_set = self._get_resource(_quota_set.QuotaSet, None, **attrs)
+        return quota_set.commit(self, **query)
 
     # ========== Server actions ==========
 

openstack/tests/unit/block_storage/v2/test_proxy.py

@@ -21,7 +21,8 @@
 from openstack.block_storage.v2 import stats
 from openstack.block_storage.v2 import type
 from openstack.block_storage.v2 import volume
-from openstack import resource
+from openstack.identity.v3 import project
+from openstack import proxy as proxy_base
 from openstack.tests.unit import test_proxy_base
 
 
@@ -526,19 +527,17 @@ def test_quota_set_reset(self):
             expected_kwargs={'user_id': 'uid'},
         )
 
-    @mock.patch('openstack.proxy.Proxy._get_resource', autospec=True)
-    def test_quota_set_update(self, gr_mock):
-        gr_mock.return_value = resource.Resource()
-        gr_mock.commit = mock.Mock()
+    @mock.patch.object(proxy_base.Proxy, '_get_resource')
+    def test_quota_set_update(self, mock_get):
+        fake_project = project.Project(id='prj')
+        mock_get.side_effect = [fake_project]
+
         self._verify(
-            'openstack.resource.Resource.commit',
+            'openstack.proxy.Proxy._update',
             self.proxy.update_quota_set,
-            method_args=['qs'],
-            method_kwargs={
-                'query': {'user_id': 'uid'},
-                'a': 'b',
-            },
-            expected_args=[self.proxy],
-            expected_kwargs={'user_id': 'uid'},
+            method_args=['prj'],
+            method_kwargs={'volumes': 123},
+            expected_args=[quota_set.QuotaSet, None],
+            expected_kwargs={'project_id': 'prj', 'volumes': 123},
         )
-        gr_mock.assert_called_with(self.proxy, quota_set.QuotaSet, 'qs', a='b')
+        mock_get.assert_called_once_with(project.Project, 'prj')

openstack/tests/unit/block_storage/v3/test_proxy.py

@@ -28,7 +28,8 @@
 from openstack.block_storage.v3 import stats
 from openstack.block_storage.v3 import type
 from openstack.block_storage.v3 import volume
-from openstack import resource
+from openstack.identity.v3 import project
+from openstack import proxy as proxy_base
 from openstack.tests.unit import test_proxy_base
 
 
@@ -1017,19 +1018,17 @@ def test_quota_set_reset(self):
             expected_kwargs={'user_id': 'uid'},
         )
 
-    @mock.patch('openstack.proxy.Proxy._get_resource', autospec=True)
-    def test_quota_set_update(self, gr_mock):
-        gr_mock.return_value = resource.Resource()
-        gr_mock.commit = mock.Mock()
+    @mock.patch.object(proxy_base.Proxy, '_get_resource')
+    def test_quota_set_update(self, mock_get):
+        fake_project = project.Project(id='prj')
+        mock_get.side_effect = [fake_project]
+
         self._verify(
-            'openstack.resource.Resource.commit',
+            'openstack.proxy.Proxy._update',
             self.proxy.update_quota_set,
-            method_args=['qs'],
-            method_kwargs={
-                'query': {'user_id': 'uid'},
-                'a': 'b',
-            },
-            expected_args=[self.proxy],
-            expected_kwargs={'user_id': 'uid'},
+            method_args=['prj'],
+            method_kwargs={'volumes': 123},
+            expected_args=[quota_set.QuotaSet, None],
+            expected_kwargs={'project_id': 'prj', 'volumes': 123},
         )
-        gr_mock.assert_called_with(self.proxy, quota_set.QuotaSet, 'qs', a='b')
+        mock_get.assert_called_once_with(project.Project, 'prj')

openstack/tests/unit/compute/v2/test_proxy.py

@@ -38,7 +38,8 @@
 from openstack.compute.v2 import service
 from openstack.compute.v2 import usage
 from openstack.compute.v2 import volume_attachment
-from openstack import resource
+from openstack.identity.v3 import project
+from openstack import proxy as proxy_base
 from openstack.tests.unit import test_proxy_base
 from openstack import warnings as os_warnings
 
@@ -1685,22 +1686,26 @@ def test_quota_set_reset(self):
             expected_kwargs={'user_id': 'uid'},
         )
 
-    @mock.patch('openstack.proxy.Proxy._get_resource', autospec=True)
-    def test_quota_set_update(self, gr_mock):
-        gr_mock.return_value = resource.Resource()
-        gr_mock.commit = mock.Mock()
+    @mock.patch.object(proxy_base.Proxy, "_get_resource")
+    def test_quota_set_update(self, mock_get):
+        fake_project = project.Project(id='prj')
+        fake_quota_set = quota_set.QuotaSet(project_id='prj')
+        mock_get.side_effect = [fake_project, fake_quota_set]
+
         self._verify(
             'openstack.resource.Resource.commit',
             self.proxy.update_quota_set,
-            method_args=['qs'],
-            method_kwargs={
-                'query': {'user_id': 'uid'},
-                'a': 'b',
-            },
+            method_args=['prj'],
+            method_kwargs={'ram': 123},
             expected_args=[self.proxy],
-            expected_kwargs={'user_id': 'uid'},
+            expected_kwargs={},
+        )
+        mock_get.assert_has_calls(
+            [
+                mock.call(project.Project, 'prj'),
+                mock.call(quota_set.QuotaSet, None, project_id='prj', ram=123),
+            ]
         )
-        gr_mock.assert_called_with(self.proxy, quota_set.QuotaSet, 'qs', a='b')
 
 
 class TestServerAction(TestComputeProxy):