From 8e3d429dea2486cce9476f0253cbc76ae4dca3b9 Mon Sep 17 00:00:00 2001 From: mgcam Date: Tue, 16 Jul 2024 14:52:24 +0100 Subject: [PATCH 1/3] Expanded error message when possible --- src/npg_porch_cli/api.py | 13 ++++++++++++- tests/test_send_request.py | 28 +++++++++++++++++++++++++--- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/src/npg_porch_cli/api.py b/src/npg_porch_cli/api.py index 211846e..792256f 100644 --- a/src/npg_porch_cli/api.py +++ b/src/npg_porch_cli/api.py @@ -319,9 +319,20 @@ def send_request( response = requests.request(method, url, **request_args) if not response.ok: - raise ServerErrorException( + detail = "" + try: + data = response.json() + if "detail" in data: + detail = data["detail"] + except Exception: + pass + + message = ( f'Status code {response.status_code} "{response.reason}" ' f"received from {response.url}" ) + if detail: + message += f".\nDetail: {detail}" + raise ServerErrorException(message) return response.json() diff --git a/tests/test_send_request.py b/tests/test_send_request.py index 70e4d82..3a37ae3 100644 --- a/tests/test_send_request.py +++ b/tests/test_send_request.py @@ -1,6 +1,5 @@ import pytest import requests - from npg_porch_cli import send_request from npg_porch_cli.api import AuthException, ServerErrorException @@ -28,7 +27,18 @@ def __init__(self): self.ok = False def json(self): - return {"Error": "Not found"} + return {"detail": "Not found in our data"} + + +class MockResponseNotFoundShort: + def __init__(self): + self.status_code = 404 + self.reason = "NOT FOUND" + self.url = url + self.ok = False + + def json(self): + return {} def mock_get_200(*args, **kwargs): @@ -39,8 +49,11 @@ def mock_get_404(*args, **kwargs): return MockResponseNotFound() -def test_sending_request(monkeypatch): +def mock_get_404_short(*args, **kwargs): + return MockResponseNotFoundShort() + +def test_sending_request(monkeypatch): monkeypatch.delenv(var_name, raising=False) with pytest.raises(ValueError) as e: @@ -66,6 +79,15 @@ def test_sending_request(monkeypatch): with monkeypatch.context() as m: m.setattr(requests, "request", mock_get_404) + with pytest.raises(ServerErrorException) as e: + send_request(validate_ca_cert=False, url=url, method="POST", data=json_data) + assert e.value.args[0] == ( + 'Status code 404 "NOT FOUND" received from ' + f"{url}.\nDetail: Not found in our data" + ) + + with monkeypatch.context() as m: + m.setattr(requests, "request", mock_get_404_short) with pytest.raises(ServerErrorException) as e: send_request(validate_ca_cert=False, url=url, method="POST", data=json_data) assert e.value.args[0] == f'Status code 404 "NOT FOUND" received from {url}' From 03ed48d4d293e4a73c66a322a785c258a354466d Mon Sep 17 00:00:00 2001 From: mgcam Date: Tue, 16 Jul 2024 14:55:26 +0100 Subject: [PATCH 2/3] Added a trailing slash to url to avoid a redirect --- src/npg_porch_cli/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/npg_porch_cli/api.py b/src/npg_porch_cli/api.py index 792256f..c675df7 100644 --- a/src/npg_porch_cli/api.py +++ b/src/npg_porch_cli/api.py @@ -230,7 +230,7 @@ def update_task(action: PorchAction, pipeline: Pipeline): raise TypeError(f"task_status cannot be None for action '{action.action}'") return send_request( validate_ca_cert=action.validate_ca_cert, - url=urljoin(action.porch_url, "tasks"), + url=urljoin(action.porch_url, "tasks/"), method="PUT", data={ "pipeline": asdict(pipeline), From 81caf8442ba4fc22926cc048395b964c2c7ab504 Mon Sep 17 00:00:00 2001 From: mgcam Date: Tue, 16 Jul 2024 15:04:12 +0100 Subject: [PATCH 3/3] Formatted import --- tests/test_send_request.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_send_request.py b/tests/test_send_request.py index 3a37ae3..188ceee 100644 --- a/tests/test_send_request.py +++ b/tests/test_send_request.py @@ -1,5 +1,6 @@ import pytest import requests + from npg_porch_cli import send_request from npg_porch_cli.api import AuthException, ServerErrorException