Skip to content

Catch agnostic HTTP error types across cloud_foundry_api and swap the test #24280

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mwdd146980 wants to merge 1 commit into
base: mwdd146980/httpx-migration-base
Choose a base branch
from
+34 −23
Open

Catch agnostic HTTP error types across cloud_foundry_api and swap the test #24280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cloud_foundry_api/changelog.d/22676.changed
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Catch the backend-agnostic ``HTTPError`` and ``HTTPStatusError`` types when querying the Cloud Foundry and UAA APIs so the check keeps working after the httpx migration.
16 changes: 9 additions & 7 deletions cloud_foundry_api/datadog_checks/cloud_foundry_api/cloud_foundry_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@
from typing import Any, Dict, Generator, Tuple # noqa: F401
from urllib.parse import urlparse

from requests.exceptions import HTTPError, RequestException
from requests.exceptions import HTTPError as RequestsHTTPError
from requests.exceptions import RequestException
from semver import VersionInfo

from datadog_checks.base import AgentCheck
from datadog_checks.base.errors import CheckException, ConfigurationError
from datadog_checks.base.types import Event # noqa: F401
from datadog_checks.base.utils.http_exceptions import HTTPError, HTTPStatusError
Comment thread
mwdd146980 marked this conversation as resolved.

from .constants import (
API_SERVICE_CHECK_NAME,
Expand Down Expand Up @@ -64,12 +66,12 @@ def discover_api(self):
self.log.info("Discovering Cloud Foundry API version and authentication endpoint")
try:
res = self.http.get(self._api_url)
except RequestException:
except (RequestException, HTTPError):
self.log.exception("Error connecting to the API server")
raise
try:
res.raise_for_status()
except HTTPError:
except (RequestsHTTPError, HTTPStatusError):
self.log.exception("Error querying API information: response: %s", res.text)
raise
try:
Expand Down Expand Up @@ -114,13 +116,13 @@ def get_oauth_token(self):
auth=(self._client_id, self._client_secret), # SKIP_HTTP_VALIDATION`
params={"grant_type": "client_credentials"},
)
except RequestException:
except (RequestException, HTTPError):
self.log.exception("Error connecting to the UAA server")
self.service_check(UAA_SERVICE_CHECK_NAME, CloudFoundryApiCheck.CRITICAL, tags=sc_tags)
raise
try:
res.raise_for_status()
except HTTPError:
except (RequestsHTTPError, HTTPStatusError):
self.log.exception("Error authenticating to the UAA server: response: %s", res.text)
self.service_check(UAA_SERVICE_CHECK_NAME, CloudFoundryApiCheck.CRITICAL, tags=sc_tags)
raise
Expand Down Expand Up @@ -257,14 +259,14 @@ def scroll_api_pages(self, url, params, headers):
self.log.debug("Fetching events page %s", page)
try:
res = self.http.get(url, params=params, headers=headers)
except RequestException:
except (RequestException, HTTPError):
self.log.exception("Error connecting to the Cloud Controller API: URL %s", url)
self.service_check(API_SERVICE_CHECK_NAME, CloudFoundryApiCheck.CRITICAL, tags=sc_tags)
raised = True
return
try:
res.raise_for_status()
except HTTPError:
except (RequestsHTTPError, HTTPStatusError):
self.log.exception("Error querying Cloud Controller API: URL %s - response %s", url, res.text)
self.service_check(API_SERVICE_CHECK_NAME, CloudFoundryApiCheck.CRITICAL, tags=sc_tags)
raised = True
Expand Down
40 changes: 24 additions & 16 deletions cloud_foundry_api/tests/test_cloud_foundry_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@

import mock
import pytest
from requests.exceptions import HTTPError, RequestException

from datadog_checks.base.errors import CheckException, ConfigurationError
from datadog_checks.base.utils.http_exceptions import HTTPError, HTTPStatusError
from datadog_checks.cloud_foundry_api import CloudFoundryApiCheck

from .constants import FREEZE_TIME
Expand Down Expand Up @@ -191,7 +191,7 @@ def test_scroll_events_errors(_, __, ___, ____, aggregator, instance, events_v3_
check._http = None # initialize the _http attribute for mocking

with mock.patch.object(check, "_http") as http_mock:
http_mock.get.side_effect = RequestException()
http_mock.get.side_effect = HTTPError("error")
check.scroll_events("", {}, {})
aggregator.assert_service_check(
name="cloud_foundry_api.api.can_connect",
Expand All @@ -202,7 +202,9 @@ def test_scroll_events_errors(_, __, ___, ____, aggregator, instance, events_v3_
aggregator.reset()

with mock.patch.object(check, "_http") as http_mock:
http_mock.get.return_value = mock.MagicMock(raise_for_status=mock.MagicMock(side_effect=HTTPError()))
http_mock.get.return_value = mock.MagicMock(
raise_for_status=mock.MagicMock(side_effect=HTTPStatusError("error"))
)
check.scroll_events("", {}, {})
aggregator.assert_service_check(
name="cloud_foundry_api.api.can_connect",
Expand All @@ -228,7 +230,7 @@ def test_scroll_events_errors(_, __, ___, ____, aggregator, instance, events_v3_
with mock.patch.object(check, "_http") as http_mock:
events_res_p1 = mock.MagicMock()
events_res_p1.json.return_value = events_v3_p1
http_mock.get.side_effect = (events_res_p1, RequestException())
http_mock.get.side_effect = (events_res_p1, HTTPError("error"))
dd_events = check.scroll_events("", {}, {})
aggregator.assert_service_check(
name="cloud_foundry_api.api.can_connect",
Expand Down Expand Up @@ -283,8 +285,8 @@ def test_get_oauth_token_errors(_, __, ___, aggregator, instance):
check = CloudFoundryApiCheck('cloud_foundry_api', {}, [instance])
check._http = None # initialize the _http attribute for mocking

with mock.patch.object(check, "_http") as http_mock, pytest.raises(RequestException):
http_mock.get.side_effect = RequestException()
with mock.patch.object(check, "_http") as http_mock, pytest.raises(HTTPError):
http_mock.get.side_effect = HTTPError("error")
check.get_oauth_token()
aggregator.assert_service_check(
name="cloud_foundry_api.uaa.can_authenticate",
Expand All @@ -294,8 +296,10 @@ def test_get_oauth_token_errors(_, __, ___, aggregator, instance):
)
aggregator.reset()

with mock.patch.object(check, "_http") as http_mock, pytest.raises(HTTPError):
http_mock.get.return_value = mock.MagicMock(raise_for_status=mock.MagicMock(side_effect=HTTPError()))
with mock.patch.object(check, "_http") as http_mock, pytest.raises(HTTPStatusError):
http_mock.get.return_value = mock.MagicMock(
raise_for_status=mock.MagicMock(side_effect=HTTPStatusError("error"))
)
check.get_oauth_token()
aggregator.assert_service_check(
name="cloud_foundry_api.uaa.can_authenticate",
Expand Down Expand Up @@ -347,12 +351,14 @@ def test_discover_api_errors(_, __, instance):
check = CloudFoundryApiCheck('cloud_foundry_api', {}, [instance])
check._http = None # initialize the _http attribute for mocking

with mock.patch.object(check, "_http") as http_mock, pytest.raises(RequestException):
http_mock.get.side_effect = RequestException()
with mock.patch.object(check, "_http") as http_mock, pytest.raises(HTTPError):
http_mock.get.side_effect = HTTPError("error")
check.discover_api()

with mock.patch.object(check, "_http") as http_mock, pytest.raises(HTTPError):
http_mock.get.return_value = mock.MagicMock(raise_for_status=mock.MagicMock(side_effect=HTTPError()))
with mock.patch.object(check, "_http") as http_mock, pytest.raises(HTTPStatusError):
http_mock.get.return_value = mock.MagicMock(
raise_for_status=mock.MagicMock(side_effect=HTTPStatusError("error"))
)
check.discover_api()

with mock.patch.object(check, "_http") as http_mock, pytest.raises(ValueError):
Expand Down Expand Up @@ -550,7 +556,7 @@ def test_scroll_api_pages_errors(_, __, ___, aggregator, instance):
check._http = None

with mock.patch.object(check, "_http") as http_mock:
http_mock.get.side_effect = RequestException()
http_mock.get.side_effect = HTTPError("error")
for _ in check.scroll_api_pages("", {}, {}):
pass
aggregator.assert_service_check(
Expand All @@ -568,7 +574,9 @@ def test_scroll_api_pages_errors(_, __, ___, aggregator, instance):
aggregator.reset()

with mock.patch.object(check, "_http") as http_mock:
http_mock.get.return_value = mock.MagicMock(raise_for_status=mock.MagicMock(side_effect=HTTPError()))
http_mock.get.return_value = mock.MagicMock(
raise_for_status=mock.MagicMock(side_effect=HTTPStatusError("error"))
)
for _ in check.scroll_api_pages("", {}, {}):
pass
aggregator.assert_service_check(
Expand Down Expand Up @@ -693,7 +701,7 @@ def test_get_org_name(http_mock, _, __, ___, instance, org_v2, org_v3):
headers={"Authorization": "Bearer {}".format(check._oauth_token)},
)
# Error
http_mock.get.side_effect = RequestException
http_mock.get.side_effect = HTTPError("error")
assert check.get_org_name("id_error") is None
log_mock.exception.assert_called_once()

Expand Down Expand Up @@ -731,7 +739,7 @@ def test_get_space_name(http_mock, _, __, ___, instance, space_v2, space_v3):
headers={"Authorization": "Bearer {}".format(check._oauth_token)},
)
# Error
http_mock.get.side_effect = RequestException
http_mock.get.side_effect = HTTPError("error")
assert check.get_space_name("id_error") is None
log_mock.exception.assert_called_once()

Expand Down
Loading