Skip to content

Commit

Permalink
exceptions: Deprecate http_status, request_id params
Browse files Browse the repository at this point in the history
Change-Id: I2b26ad6d96988022b5c08ae7dd6d17e4eb7ca905
Signed-off-by: Stephen Finucane <[email protected]>
  • Loading branch information
stephenfin committed Feb 26, 2025
1 parent 6438e3b commit 7fb2a33
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 41 deletions.
24 changes: 4 additions & 20 deletions openstack/cloud/_network.py
Original file line number Diff line number Diff line change
Expand Up @@ -1863,12 +1863,8 @@ def get_qos_minimum_bandwidth_rule(self, policy_name_or_id, rule_id):
)

policy = self.network.find_qos_policy(
policy_name_or_id, ignore_missing=True
policy_name_or_id, ignore_missing=False
)
if not policy:
raise exceptions.NotFoundException(
f"QoS policy {policy_name_or_id} not Found."
)

return self.network.get_qos_minimum_bandwidth_rule(rule_id, policy)

Expand Down Expand Up @@ -1897,12 +1893,8 @@ def create_qos_minimum_bandwidth_rule(
)

policy = self.network.find_qos_policy(
policy_name_or_id, ignore_missing=True
policy_name_or_id, ignore_missing=False
)
if not policy:
raise exceptions.NotFoundException(
f"QoS policy {policy_name_or_id} not Found."
)

kwargs['min_kbps'] = min_kbps

Expand Down Expand Up @@ -1931,12 +1923,8 @@ def update_qos_minimum_bandwidth_rule(
)

policy = self.network.find_qos_policy(
policy_name_or_id, ignore_missing=True
policy_name_or_id, ignore_missing=False
)
if not policy:
raise exceptions.NotFoundException(
f"QoS policy {policy_name_or_id} not Found."
)

if not kwargs:
self.log.debug("No QoS minimum bandwidth rule data to update")
Expand Down Expand Up @@ -1971,12 +1959,8 @@ def delete_qos_minimum_bandwidth_rule(self, policy_name_or_id, rule_id):
)

policy = self.network.find_qos_policy(
policy_name_or_id, ignore_missing=True
policy_name_or_id, ignore_missing=False
)
if not policy:
raise exceptions.NotFoundException(
f"QoS policy {policy_name_or_id} not Found."
)

try:
self.network.delete_qos_minimum_bandwidth_rule(
Expand Down
30 changes: 23 additions & 7 deletions openstack/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,13 @@
import json
import re
import typing as ty
import warnings

import requests
from requests import exceptions as _rex

from openstack import warnings as os_warnings

if ty.TYPE_CHECKING:
from openstack import resource

Expand Down Expand Up @@ -73,13 +76,31 @@ def __init__(
details: ty.Optional[str] = None,
request_id: ty.Optional[str] = None,
):
# TODO(shade) Remove http_status parameter and the ability for response
# to be None once we're not mocking Session everywhere.
if http_status is not None:
warnings.warn(
"The 'http_status' parameter is unnecessary and will be "
"removed in a future release",
os_warnings.RemovedInSDK50Warning,
)

if request_id is not None:
warnings.warn(
"The 'request_id' parameter is unnecessary and will be "
"removed in a future release",
os_warnings.RemovedInSDK50Warning,
)

if not message:
if response is not None:
message = f"{self.__class__.__name__}: {response.status_code}"
else:
message = f"{self.__class__.__name__}: Unknown error"
status = (
response.status_code
if response is not None
else 'Unknown error'
)
message = f'{self.__class__.__name__}: {status}'

# Call directly rather than via super to control parameters
SDKException.__init__(self, message=message)
Expand Down Expand Up @@ -241,15 +262,10 @@ def raise_from_response(
if not details:
details = response.reason if response.reason else response.text

http_status = response.status_code
request_id = response.headers.get('x-openstack-request-id')

raise cls(
message=error_message,
response=response,
details=details,
http_status=http_status,
request_id=request_id,
)


Expand Down
44 changes: 35 additions & 9 deletions openstack/tests/unit/test_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,32 +13,49 @@
import json
from unittest import mock
import uuid
import warnings

from openstack import exceptions
from openstack.tests.unit import base
from openstack.tests.unit import fakes


class Test_Exception(base.TestCase):
def test_method_not_supported(self):
exc = exceptions.MethodNotSupported(self.__class__, 'list')
expected = (
'The list method is not supported for '
+ 'openstack.tests.unit.test_exceptions.Test_Exception'
'openstack.tests.unit.test_exceptions.Test_Exception'
)
self.assertEqual(expected, str(exc))


class Test_HttpException(base.TestCase):
def setUp(self):
super().setUp()
self.message = "mayday"
self.message = 'mayday'
self.response = fakes.FakeResponse(
status_code=401,
data={
'error': {
'code': 401,
'message': (
'The request you have made requires authentication.'
),
'title': 'Unauthorized',
},
},
)

def _do_raise(self, *args, **kwargs):
raise exceptions.HttpException(*args, **kwargs)

def test_message(self):
exc = self.assertRaises(
exceptions.HttpException, self._do_raise, self.message
exceptions.HttpException,
self._do_raise,
self.message,
response=self.response,
)

self.assertEqual(self.message, exc.message)
Expand All @@ -49,6 +66,7 @@ def test_details(self):
exceptions.HttpException,
self._do_raise,
self.message,
response=self.response,
details=details,
)

Expand All @@ -57,16 +75,24 @@ def test_details(self):

def test_http_status(self):
http_status = 123
exc = self.assertRaises(
exceptions.HttpException,
self._do_raise,
self.message,
http_status=http_status,
)
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter('always')

exc = self.assertRaises(
exceptions.HttpException,
self._do_raise,
self.message,
http_status=http_status,
)

self.assertEqual(self.message, exc.message)
self.assertEqual(http_status, exc.status_code)

self.assertIn(
"The 'http_status' parameter is unnecessary",
str(w[-1]),
)


class TestRaiseFromResponse(base.TestCase):
def setUp(self):
Expand Down
15 changes: 10 additions & 5 deletions openstack/tests/unit/test_proxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from openstack import proxy
from openstack import resource
from openstack.tests.unit import base
from openstack.tests.unit import fakes
from openstack import utils


Expand Down Expand Up @@ -237,15 +238,17 @@ def test_delete(self):

def test_delete_ignore_missing(self):
self.res.delete.side_effect = exceptions.NotFoundException(
message="test", http_status=404
message="test",
response=fakes.FakeResponse(status_code=404, data={'error': None}),
)

rv = self.sot._delete(DeleteableResource, self.fake_id)
self.assertIsNone(rv)

def test_delete_NotFound(self):
self.res.delete.side_effect = exceptions.NotFoundException(
message="test", http_status=404
message="test",
response=fakes.FakeResponse(status_code=404, data={'error': None}),
)

self.assertRaisesRegex(
Expand All @@ -259,8 +262,9 @@ def test_delete_NotFound(self):
)

def test_delete_HttpException(self):
self.res.delete.side_effect = exceptions.HttpException(
message="test", http_status=500
self.res.delete.side_effect = exceptions.ResourceNotFound(
message="test",
response=fakes.FakeResponse(status_code=500, data={'error': None}),
)

self.assertRaises(
Expand Down Expand Up @@ -468,7 +472,8 @@ def test_get_base_path(self):

def test_get_not_found(self):
self.res.fetch.side_effect = exceptions.NotFoundException(
message="test", http_status=404
message="test",
response=fakes.FakeResponse(status_code=404, data={'error': None}),
)

self.assertRaisesRegex(
Expand Down

0 comments on commit 7fb2a33

Please sign in to comment.