Merge "compute, volume: Tweak 'update_quota_set' compat shim"

openstack/block_storage/v2/_proxy.py

@@ -824,18 +824,22 @@ class Proxy(_base_proxy.BaseBlockStorageProxy):
                 "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')
+                warnings.warn(
+                    "The query argument is no longer supported and should "
+                    "be removed.",
+                    os_warnings.RemovedInSDK50Warning,
+                )
+                attrs.pop('query')
+
+            res = self._get_resource(_quota_set.QuotaSet, project, **attrs)
+            return res.commit(self)
         else:
             project = self._get_resource(_project.Project, project)
             attrs['project_id'] = project.id
-
+            return self._update(_quota_set.QuotaSet, None, **attrs)
-        return self._update(_quota_set.QuotaSet, None, **attrs)
 
     # ====== VOLUME METADATA ======
     def get_volume_metadata(self, volume):

openstack/block_storage/v3/_proxy.py

@@ -1898,18 +1898,22 @@ class Proxy(_base_proxy.BaseBlockStorageProxy):
                 "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')
+                warnings.warn(
+                    "The query argument is no longer supported and should "
+                    "be removed.",
+                    os_warnings.RemovedInSDK50Warning,
+                )
+                attrs.pop('query')
+
+            res = self._get_resource(_quota_set.QuotaSet, project, **attrs)
+            return res.commit(self)
         else:
             project = self._get_resource(_project.Project, project)
             attrs['project_id'] = project.id
-
+            return self._update(_quota_set.QuotaSet, None, **attrs)
-        return self._update(_quota_set.QuotaSet, None, **attrs)
 
     # ====== SERVICES ======
     @ty.overload

openstack/compute/v2/_proxy.py

@@ -2458,7 +2458,7 @@ class Proxy(proxy.Proxy):
     # ========== Quota sets ==========
 
     def get_quota_set(self, project, usage=False, **query):
-        """Show QuotaSet information for the project
+        """Show QuotaSet information for the project.
 
         :param project: ID or instance of
             :class:`~openstack.identity.project.Project` of the project for
@@ -2481,7 +2481,7 @@ class Proxy(proxy.Proxy):
         return res.fetch(self, base_path=base_path, **query)
 
     def get_quota_set_defaults(self, project):
-        """Show QuotaSet defaults for the project
+        """Show QuotaSet defaults for the project.
 
         :param project: ID or instance of
             :class:`~openstack.identity.project.Project` of the project for
@@ -2533,8 +2533,6 @@ class Proxy(proxy.Proxy):
         :returns: The updated QuotaSet
         :rtype: :class:`~openstack.compute.v2.quota_set.QuotaSet`
         """
-        query = {}
-
         if 'project_id' in attrs or isinstance(project, _quota_set.QuotaSet):
             warnings.warn(
                 "The signature of 'update_quota_set' has changed and it "
@@ -2542,23 +2540,38 @@ class Proxy(proxy.Proxy):
                 "with the other quota set methods.",
                 os_warnings.RemovedInSDK50Warning,
             )
-            if isinstance(project, _quota_set.QuotaSet):
+            if user is not None:
-                attrs['project_id'] = project.project_id
+                raise exceptions.SDKException(
+                    'The user argument can only be provided once the entire '
+                    'call has been updated.'
+                )
 
             if 'query' in attrs:
-                query = attrs.pop('query')
+                warnings.warn(
+                    "The query argument is no longer supported and should "
+                    "be removed.",
+                    os_warnings.RemovedInSDK50Warning,
+                )
+                query = attrs.pop('query') or {}
+            else:
+                query = {}
+
+            res = self._get_resource(_quota_set.QuotaSet, project, **attrs)
+            return res.commit(self, **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
+                query = {'user_id': user.id}
+            else:
+                query = {}
 
-        # we don't use Proxy._update since that doesn't allow passing arbitrary
+            # we don't use Proxy._update since that doesn't allow passing
-        # query string parameters
+            # arbitrary query string parameters
-        quota_set = self._get_resource(_quota_set.QuotaSet, None, **attrs)
+            quota_set = self._get_resource(_quota_set.QuotaSet, None, **attrs)
-        return quota_set.commit(self, **query)
+            return quota_set.commit(self, **query)
 
     # ========== Server actions ==========
 

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

@@ -9,7 +9,9 @@
 # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 # License for the specific language governing permissions and limitations
 # under the License.
+
 from unittest import mock
+import warnings
 
 from openstack.block_storage.v2 import _proxy
 from openstack.block_storage.v2 import backup
@@ -24,6 +26,7 @@ from openstack.block_storage.v2 import volume
 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
 
 
 class TestVolumeProxy(test_proxy_base.TestProxyBase):
@@ -577,3 +580,29 @@ class TestQuotaSet(TestVolumeProxy):
             expected_kwargs={'project_id': 'prj', 'volumes': 123},
         )
         mock_get.assert_called_once_with(project.Project, 'prj')
+
+    @mock.patch.object(proxy_base.Proxy, "_get_resource")
+    def test_quota_set_update__legacy(self, mock_get):
+        fake_quota_set = quota_set.QuotaSet(project_id='prj')
+        mock_get.side_effect = [fake_quota_set]
+        with warnings.catch_warnings(record=True) as w:
+            warnings.simplefilter('always')
+
+            self._verify(
+                'openstack.resource.Resource.commit',
+                self.proxy.update_quota_set,
+                method_args=[fake_quota_set],
+                method_kwargs={'ram': 123},
+                expected_args=[self.proxy],
+                expected_kwargs={},
+            )
+
+            self.assertEqual(1, len(w))
+            self.assertEqual(
+                os_warnings.RemovedInSDK50Warning,
+                w[-1].category,
+            )
+            self.assertIn(
+                "The signature of 'update_quota_set' has changed ",
+                str(w[-1]),
+            )

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

@@ -11,6 +11,7 @@
 # under the License.
 
 from unittest import mock
+import warnings
 
 from openstack.block_storage.v3 import _proxy
 from openstack.block_storage.v3 import backup
@@ -30,6 +31,7 @@ from openstack.block_storage.v3 import volume
 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
 
 
 class TestVolumeProxy(test_proxy_base.TestProxyBase):
@@ -1050,3 +1052,29 @@ class TestQuotaSet(TestVolumeProxy):
             expected_kwargs={'project_id': 'prj', 'volumes': 123},
         )
         mock_get.assert_called_once_with(project.Project, 'prj')
+
+    @mock.patch.object(proxy_base.Proxy, "_get_resource")
+    def test_quota_set_update__legacy(self, mock_get):
+        fake_quota_set = quota_set.QuotaSet(project_id='prj')
+        mock_get.side_effect = [fake_quota_set]
+        with warnings.catch_warnings(record=True) as w:
+            warnings.simplefilter('always')
+
+            self._verify(
+                'openstack.resource.Resource.commit',
+                self.proxy.update_quota_set,
+                method_args=[fake_quota_set],
+                method_kwargs={'ram': 123},
+                expected_args=[self.proxy],
+                expected_kwargs={},
+            )
+
+            self.assertEqual(1, len(w))
+            self.assertEqual(
+                os_warnings.RemovedInSDK50Warning,
+                w[-1].category,
+            )
+            self.assertIn(
+                "The signature of 'update_quota_set' has changed ",
+                str(w[-1]),
+            )

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

@@ -1744,6 +1744,32 @@ class TestQuotaSet(TestComputeProxy):
             ]
         )
 
+    @mock.patch.object(proxy_base.Proxy, "_get_resource")
+    def test_quota_set_update__legacy(self, mock_get):
+        fake_quota_set = quota_set.QuotaSet(project_id='prj')
+        mock_get.side_effect = [fake_quota_set]
+        with warnings.catch_warnings(record=True) as w:
+            warnings.simplefilter('always')
+
+            self._verify(
+                'openstack.resource.Resource.commit',
+                self.proxy.update_quota_set,
+                method_args=[fake_quota_set],
+                method_kwargs={'ram': 123},
+                expected_args=[self.proxy],
+                expected_kwargs={},
+            )
+
+            self.assertEqual(1, len(w))
+            self.assertEqual(
+                os_warnings.RemovedInSDK50Warning,
+                w[-1].category,
+            )
+            self.assertIn(
+                "The signature of 'update_quota_set' has changed ",
+                str(w[-1]),
+            )
+
 
 class TestServerAction(TestComputeProxy):
     def test_server_action_get(self):

releasenotes/notes/bug-2081292-def552ed9c4e24a3.yaml

@@ -0,0 +1,10 @@
+---
+fixes:
+  - |
+    The ``update_quota_set`` methods in the Compute and Block Storage (v2, v3)
+    proxy APIs were modified in v3.3.0 to accept ``Project`` objects as the
+    first argument. A compatibility shim was included to handle callers still
+    passing ``QuotaSet`` objects, but this shim did not modify the provided
+    ``QuotaSet`` object in place as the previous code did. This has now been
+    fixed. The shim is still expected to be removed in v5.0.0.
+    [`bug 2081292 <https://bugs.launchpad.net/openstacksdk/+bug/2081292>`_]