Skip to content
Merged

Ruff #35

Show file tree
Hide file tree
Changes from 2 commits
Commits
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Updated Google CSP domains
- Switched to checking response codes from `requests` in `SimpleJsonApiClient`
- Renamed `ResourceForbidden`, `ResourceNotFound` and `ResourceUnauthorized` to `ResourceForbiddenError`, `ResourceNotFoundError` and `ResourceUnauthorizedError` in `tna_utilities.api`
- `SimpleJsonApiClient` raises `ApiError` exceptions rather than generic `Exception`
- Renamed the `bytes` parameter of `pretty_file_size()` to `filesize_bytes`

### Deprecated

Expand All @@ -27,6 +30,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed

- Fixed logic inversion for `default_headers` in `SimpleJsonApiClient`
- Incorrect `ValueError` exceptions changed to `TypeError`

### Security

Expand Down
6 changes: 3 additions & 3 deletions docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ except Exception as error:

You can catch and handle some of the more common exceptions:

- `tna_utilities.api.ResourceForbidden`
- `tna_utilities.api.ResourceNotFound`
- `tna_utilities.api.ResourceUnauthorized`
- `tna_utilities.api.ResourceForbiddenError`
- `tna_utilities.api.ResourceNotFoundError`
- `tna_utilities.api.ResourceUnauthorizedError`

You can also catch [exceptions raised by `requests`](https://requests.readthedocs.io/en/latest/_modules/requests/exceptions/).

Expand Down
8 changes: 4 additions & 4 deletions docs/number.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ Formats file size to a human-readable string.

### Arguments

| Argument | Description | Default |
| ---------- | ---------------------------------------------------------- | ------- |
| `bytes` | The number of bytes | [none] |
| `simplify` | If `True`, simplify the string and remove fractional sizes | `True` |
| Argument | Description | Default |
| ---------------- | ---------------------------------------------------------- | ------- |
| `filesize_bytes` | The number of bytes | [none] |
| `simplify` | If `True`, simplify the string and remove fractional sizes | `True` |

### Example

Expand Down
9 changes: 9 additions & 0 deletions ruff.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
extend = "/home/app/ruff-strict.toml"

[lint]
ignore = [
"TRY003",
"PLR0913",
"PLR0911",
"PLR0912"
]
3 changes: 2 additions & 1 deletion tasks/checkformat.sh
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#!/bin/bash

docker run --rm -v "$(pwd)":/app/ ghcr.io/nationalarchives/tna-python-dev:preview checkformat
# TODO: Change ruff version back to preview once the Docker image has been released
Comment thread
ahosgood marked this conversation as resolved.
Outdated
docker run --rm -v "$(pwd)":/app/ ghcr.io/nationalarchives/tna-python-dev:ruff checkformat
3 changes: 2 additions & 1 deletion tasks/format.sh
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#!/bin/bash

docker run --rm -v "$(pwd)":/app/ ghcr.io/nationalarchives/tna-python-dev:preview format
# TODO: Change ruff version back to preview once the Docker image has been released
Comment thread
ahosgood marked this conversation as resolved.
Outdated
docker run --rm -v "$(pwd)":/app/ ghcr.io/nationalarchives/tna-python-dev:ruff format
28 changes: 15 additions & 13 deletions tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@
from unittest import TestCase, mock

from requests import Timeout

from tna_utilities.api import (
ResourceForbidden,
ResourceNotFound,
ResourceUnauthorized,
ApiError,
ResourceForbiddenError,
ResourceNotFoundError,
ResourceUnauthorizedError,
SimpleJsonApiClient,
)

Expand All @@ -29,15 +31,15 @@ def json(self):

if args[0] == f"{MOCK_API_BASE_URL}happy":
return MockResponse({"foo": "bar"}, 200)
elif args[0] == f"{MOCK_API_BASE_URL}badrequest":
if args[0] == f"{MOCK_API_BASE_URL}badrequest":
return MockResponse(None, 400)
elif args[0] == f"{MOCK_API_BASE_URL}unauthorized":
if args[0] == f"{MOCK_API_BASE_URL}unauthorized":
return MockResponse(None, 401)
elif args[0] == f"{MOCK_API_BASE_URL}forbidden":
if args[0] == f"{MOCK_API_BASE_URL}forbidden":
return MockResponse(None, 403)
elif args[0] == f"{MOCK_API_BASE_URL}servererror":
if args[0] == f"{MOCK_API_BASE_URL}servererror":
return MockResponse(None, 500)
elif args[0] == f"{MOCK_API_BASE_URL}timeout":
if args[0] == f"{MOCK_API_BASE_URL}timeout":
raise Timeout("Request timed out")

return MockResponse(None, 404)
Expand Down Expand Up @@ -80,28 +82,28 @@ def test_happy(self, mock_post, mock_get):
@mock.patch("requests.post", side_effect=mocked_requests_post)
def test_bad_request(self, mock_post, mock_get):
client = SimpleJsonApiClient(MOCK_API_BASE_URL)
with self.assertRaises(Exception):
with self.assertRaises(ApiError):
client.get("/badrequest")

@mock.patch("requests.get", side_effect=mocked_requests_get)
@mock.patch("requests.post", side_effect=mocked_requests_post)
def test_not_found(self, mock_post, mock_get):
client = SimpleJsonApiClient(MOCK_API_BASE_URL)
with self.assertRaises(ResourceNotFound):
with self.assertRaises(ResourceNotFoundError):
client.get("/notfound")

@mock.patch("requests.get", side_effect=mocked_requests_get)
@mock.patch("requests.post", side_effect=mocked_requests_post)
def test_resource_unauthorized(self, mock_post, mock_get):
client = SimpleJsonApiClient(MOCK_API_BASE_URL)
with self.assertRaises(ResourceUnauthorized):
with self.assertRaises(ResourceUnauthorizedError):
client.get("/unauthorized")

@mock.patch("requests.get", side_effect=mocked_requests_get)
@mock.patch("requests.post", side_effect=mocked_requests_post)
def test_resource_forbidden(self, mock_post, mock_get):
client = SimpleJsonApiClient(MOCK_API_BASE_URL)
with self.assertRaises(ResourceForbidden):
with self.assertRaises(ResourceForbiddenError):
client.get("/forbidden")

@mock.patch("requests.get", side_effect=mocked_requests_get)
Expand All @@ -115,7 +117,7 @@ def test_resource_timeout(self, mock_post, mock_get):
@mock.patch("requests.post", side_effect=mocked_requests_post)
def test_other_exception(self, mock_post, mock_get):
client = SimpleJsonApiClient(MOCK_API_BASE_URL)
with self.assertRaises(Exception):
with self.assertRaises(ApiError):
client.get("/servererror")

@mock.patch("requests.get", side_effect=mocked_requests_get)
Expand Down
8 changes: 4 additions & 4 deletions tests/test_component.py
Original file line number Diff line number Diff line change
Expand Up @@ -309,19 +309,19 @@ def test_pagination_third_no_around(self):
)

def test_pagination_no_pages(self):
with self.assertRaises(Exception):
with self.assertRaises(ValueError):
paginate(0, 1)

def test_pagination_invalid_pages(self):
with self.assertRaises(Exception):
with self.assertRaises(TypeError):
paginate(None, 1)

def test_pagination_negative_current_page(self):
with self.assertRaises(Exception):
with self.assertRaises(ValueError):
paginate(42, -1)

def test_pagination_negative_around(self):
with self.assertRaises(Exception):
with self.assertRaises(ValueError):
paginate(42, 1, around=-1)

def test_tna_pagination_items(self):
Expand Down
1 change: 1 addition & 0 deletions tests/test_flask_talisman.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import unittest

from flask import Flask, session

from tna_utilities.flask.talisman import Talisman


Expand Down
20 changes: 9 additions & 11 deletions tests/test_number.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@


class TestNumberish(unittest.TestCase):

def test_happy(self):
self.assertEqual(numberish(0), "None")
self.assertEqual(numberish(1), "1")
Expand Down Expand Up @@ -86,18 +85,17 @@ def test_happy_prefix_tuple(self):
)

def test_unhappy(self):
with self.assertRaises(ValueError):
with self.assertRaises(TypeError):
numberish("one")
with self.assertRaises(ValueError):
with self.assertRaises(TypeError):
numberish(None)
with self.assertRaises(ValueError):
with self.assertRaises(TypeError):
numberish({})
with self.assertRaises(ValueError):
with self.assertRaises(TypeError):
numberish([])


class TestPrettyFileSize(unittest.TestCase):

def test_pretty_file_size(self):
self.assertEqual(pretty_file_size(0), "0B")
self.assertEqual(pretty_file_size(999), "999B")
Expand Down Expand Up @@ -139,13 +137,13 @@ def test_pretty_file_size_unsimplified(self):
self.assertEqual(pretty_file_size(1000000000000000, simplify=False), "1PB")

def test_pretty_file_size_unhappy(self):
with self.assertRaises(ValueError):
with self.assertRaises(TypeError):
pretty_file_size(1.234)
with self.assertRaises(ValueError):
with self.assertRaises(TypeError):
pretty_file_size("one")
with self.assertRaises(ValueError):
with self.assertRaises(TypeError):
pretty_file_size(None)
with self.assertRaises(ValueError):
with self.assertRaises(TypeError):
pretty_file_size({})
with self.assertRaises(ValueError):
with self.assertRaises(TypeError):
pretty_file_size([])
3 changes: 1 addition & 2 deletions tests/test_security.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,8 @@


class TestSecurityCSP(unittest.TestCase):

def __init__(self, *args, **kwargs):
super(TestSecurityCSP, self).__init__(*args, **kwargs)
super().__init__(*args, **kwargs)
self.test_domain = "https://example.com"
self.test_domain_2 = "https://another.net"

Expand Down
3 changes: 1 addition & 2 deletions tests/test_url.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,8 @@


class TestQueryStringTransformer(unittest.TestCase):

def __init__(self, *args, **kwargs):
super(TestQueryStringTransformer, self).__init__(*args, **kwargs)
super().__init__(*args, **kwargs)
self.test_query = [("a", ["1"]), ("b", ["2", "3"])]

def test_init(self):
Expand Down
6 changes: 3 additions & 3 deletions tna_utilities/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ def strtobool(value: str) -> bool:
"""
try:
value = value.lower()
except AttributeError:
raise TypeError("invalid truth value %r" % (value,))
except AttributeError as e:
raise TypeError(f"invalid truth value {value!r}") from e

if value in ("y", "yes", "t", "true", "on", "1"):
return True
if value in ("n", "no", "f", "false", "off", "0"):
return False
raise ValueError("invalid truth value %r" % (value,))
raise ValueError(f"invalid truth value {value!r}")
54 changes: 28 additions & 26 deletions tna_utilities/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,19 @@
from requests import JSONDecodeError, Response, codes


class ResourceNotFound(Exception):
class ApiError(Exception):
pass


class ResourceForbidden(Exception):
class ResourceNotFoundError(ApiError):
pass


class ResourceUnauthorized(Exception):
class ResourceForbiddenError(ApiError):
pass


class ResourceUnauthorizedError(ApiError):
pass


Expand Down Expand Up @@ -99,9 +103,9 @@ def get(
:param params: Optional dictionary of query parameters to include in the request. These will be merged with any default parameters set for the client.
:param headers: Optional dictionary of headers to include in the request. These will be merged with any default headers set for the client.
:param timeout: Timeout in seconds for the request. Defaults to 10.
:raises ResourceNotFound: If the requested resource is not found (HTTP 404).
:raises ResourceForbidden: If access to the resource is forbidden (HTTP 403).
:raises ResourceUnauthorized: If authentication is required or has failed (HTTP 401).
:raises ResourceNotFoundError: If the requested resource is not found (HTTP 404).
:raises ResourceForbiddenError: If access to the resource is forbidden (HTTP 403).
:raises ResourceUnauthorizedError: If authentication is required or has failed (HTTP 401).
:raises Exception: For unexpected response handling or request-processing errors.
"""

Expand All @@ -112,7 +116,7 @@ def get(
headers=self.headers if headers is None else {**self.headers, **headers},
timeout=timeout,
)
return self._handle_response(response)
return self._handle_response(response, path)

def post(
self,
Expand All @@ -133,9 +137,9 @@ def post(
:param params: Optional dictionary of query parameters to include in the request. These will be merged with any default parameters set for the client.
:param headers: Optional dictionary of headers to include in the request. These will be merged with any default headers set for the client.
:param timeout: Request timeout in seconds.
:raises ResourceNotFound: If the requested resource is not found (HTTP 404).
:raises ResourceForbidden: If access to the resource is forbidden (HTTP 403).
:raises ResourceUnauthorized: If authentication is required or has failed (HTTP 401).
:raises ResourceNotFoundError: If the requested resource is not found (HTTP 404).
:raises ResourceForbiddenError: If access to the resource is forbidden (HTTP 403).
:raises ResourceUnauthorizedError: If authentication is required or has failed (HTTP 401).
:raises Exception: For other non-success responses or unexpected response handling errors.
"""

Expand All @@ -148,37 +152,35 @@ def post(
json=json,
timeout=timeout,
)
return self._handle_response(response)
return self._handle_response(response, path)

def _handle_response(self, response: Response) -> dict:
def _handle_response(self, response: Response, path: str) -> dict:
"""
Handle the API response, checking for common HTTP status codes and returning the JSON content if the request was successful.
"""

if response.status_code == codes.ok:
try:
return response.json()
except JSONDecodeError:
raise Exception("Non-JSON response provided")
except JSONDecodeError as e:
raise ApiError("Non-JSON response provided") from e
if response.status_code == codes.bad_request:
try:
error_body = response.json()
except JSONDecodeError:
raise ApiError(f"Bad request: {error_body}")
except JSONDecodeError as e:
error_body = response.text
raise Exception(f"Bad request: {error_body}")
raise ApiError(f"Bad request: {error_body}") from e
if response.status_code == codes.unauthorized:
raise ResourceUnauthorized("Unauthorized")
raise ResourceUnauthorizedError("Unauthorized")
if response.status_code == codes.forbidden:
raise ResourceForbidden("Forbidden")
raise ResourceForbiddenError("Forbidden")
if response.status_code == codes.not_found:
raise ResourceNotFound("Resource not found")
body_preview = (response.text or "").strip()
raise ResourceNotFoundError("Resource not found")
body_preview = (response.text if hasattr(response, "text") else "").strip()
if body_preview:
body_preview = body_preview[:500]
raise Exception(
f"Request failed with status {response.status_code} for URL {response.url}. "
f"Response body: {body_preview}"
raise ApiError(
f"Request failed with status {response.status_code} for {path}. Response body: {body_preview}"
)
raise Exception(
f"Request failed with status {response.status_code} for URL {response.url}"
)
raise ApiError(f"Request failed with status {response.status_code} for {path}")
Loading
Loading