Merge pull request #226 from alphagov/SW-formatting

Apply our formatting rules and tweak log messages

.pre-commit-config.yaml

@@ -6,16 +6,11 @@ repos:
   - id: end-of-file-fixer
   - id: check-yaml
   - id: debug-statements
-- repo: https://github.com/PyCQA/flake8
-  rev: 5.0.4
+- repo: https://github.com/charliermarsh/ruff-pre-commit
+  rev: 'v0.0.261'
   hooks:
-  - id: flake8
-    name: flake8 (python)
-- repo: https://github.com/PyCQA/isort
-  rev: 5.12.0
-  hooks:
-    - id: isort
-      name: isort (python)
+    - id: ruff
+      args: [--fix, --exit-non-zero-on-fix]
 - repo: https://github.com/psf/black
   rev: 22.8.0
   hooks:

CHANGELOG.md

@@ -1,3 +1,9 @@
+### 8.0.1
+
+* Some minor non-functional code reformatting.
+* Changing emitted logs to use lazy %-based evaluation rather than upfront "".format
+* Changing an error-level log to a warning-level log (request failures). These still throw the actual exception to be caught and handled by the calling code.
+
 ### 8.0.0
 
 * Add support for python 3.10 and 3.11

Makefile

@@ -14,8 +14,7 @@ build: bootstrap ## Build project (dummy task for CI)
 
 .PHONY: test
 test: ## Run tests
-	flake8 .
-	isort --check-only ./notifications_python_client ./utils ./integration_test ./tests
+	ruff check .
 	black --check .
 	pytest
 

notifications_python_client/__init__.py

@@ -7,7 +7,7 @@
 #
 # -- http://semver.org/
 
-__version__ = "8.0.0"
+__version__ = "8.0.1"
 
 from notifications_python_client.errors import (  # noqa
     REQUEST_ERROR_MESSAGE,

notifications_python_client/authentication.py

@@ -67,8 +67,8 @@ def get_token_issuer(token):
             raise TokenIssuerError
 
         return unverified.get("iss")
-    except jwt.DecodeError:
-        raise TokenDecodeError
+    except jwt.DecodeError as e:
+        raise TokenDecodeError from e
 
 
 def decode_jwt_token(token, secret):
@@ -94,22 +94,22 @@ def decode_jwt_token(token, secret):
             token, key=secret, options={"verify_signature": True}, algorithms=[__algorithm__], leeway=__bound__
         )
         return validate_jwt_token(decoded_token)
-    except jwt.InvalidIssuedAtError:
-        raise TokenExpiredError("Token has invalid iat field", decode_token(token))
-    except jwt.ImmatureSignatureError:
-        raise TokenExpiredError(INVALID_FUTURE_TOKEN_ERROR_MESSAGE, decode_token(token))
-    except jwt.DecodeError:
-        raise TokenDecodeError
-    except jwt.InvalidAlgorithmError:
-        raise TokenAlgorithmError
-    except jwt.InvalidTokenError:
+    except jwt.InvalidIssuedAtError as e:
+        raise TokenExpiredError("Token has invalid iat field", decode_token(token)) from e
+    except jwt.ImmatureSignatureError as e:
+        raise TokenExpiredError(INVALID_FUTURE_TOKEN_ERROR_MESSAGE, decode_token(token)) from e
+    except jwt.DecodeError as e:
+        raise TokenDecodeError from e
+    except jwt.InvalidAlgorithmError as e:
+        raise TokenAlgorithmError from e
+    except jwt.InvalidTokenError as e:
         # At this point, we have not caught a specific exception we care about enough to show
         # a precise error message (ie something to do with the iat, iss or alg fields).
         # If there is a different reason our token is invalid we will throw a generic error as we
         # don't wish to provide exact messages for every type of error that jwt might encounter.
         # https://github.com/jpadilla/pyjwt/blob/master/jwt/exceptions.py
         # https://pyjwt.readthedocs.io/en/latest/api.html#exceptions
-        raise TokenError
+        raise TokenError from e
 
 
 def validate_jwt_token(decoded_token):

notifications_python_client/base.py

@@ -53,7 +53,7 @@ def generate_headers(self, api_token):
         }
 
     def request(self, method, url, data=None, params=None):
-        logger.debug("API request {} {}".format(method, url))
+        logger.debug("API request %s %s", method, url)
         url, kwargs = self._create_request_objects(url, data, params)
 
         response = self._perform_request(method, url, kwargs)
@@ -92,18 +92,18 @@ def _perform_request(self, method, url, kwargs):
             return response
         except requests.RequestException as e:
             api_error = HTTPError.create(e)
-            logger.error(
-                "API {} request on {} failed with {} '{}'".format(method, url, api_error.status_code, api_error.message)
+            logger.warning(
+                "API %s request on %s failed with %s '%s'", method, url, api_error.status_code, api_error.message
             )
             raise api_error
         finally:
             elapsed_time = time.monotonic() - start_time
-            logger.debug("API {} request on {} finished in {}".format(method, url, elapsed_time))
+            logger.debug("API %s request on %s finished in %s", method, url, elapsed_time)
 
     def _process_json_response(self, response):
         try:
             if response.status_code == 204:
                 return
             return response.json()
-        except ValueError:
-            raise InvalidResponse(response, message="No JSON response object could be decoded")
+        except ValueError as e:
+            raise InvalidResponse(response, message="No JSON response object could be decoded") from e

notifications_python_client/notifications.py

@@ -72,7 +72,7 @@ def get_notification_by_id(self, id):
 
     def get_pdf_for_letter(self, id):
         url = "/v2/notifications/{}/pdf".format(id)
-        logger.debug("API request {} {}".format("GET", url))
+        logger.debug("API request %s %s", "GET", url)
         url, kwargs = self._create_request_objects(url, data=None, params=None)
 
         response = self._perform_request("GET", url, kwargs)

pyproject.toml

@@ -1,5 +1,24 @@
 [tool.black]
 line-length = 120
 
-[tool.isort]
-profile = "black"
+[tool.ruff]
+line-length = 120
+
+target-version = "py39"
+
+select = [
+    "E",  # pycodestyle
+    "W",  # pycodestyle
+    "F",  # pyflakes
+    "I",  # isort
+    "B",  # flake8-bugbear
+    "C90",  # mccabe cyclomatic complexity
+    "G",  # flake8-logging-format
+]
+ignore = []
+exclude = [
+    "venv*",
+    "__pycache__",
+    "cache",
+    "build",
+]

requirements_for_test.txt

@@ -1,8 +1,5 @@
 .
 
-isort==5.12.0 ; python_version >= '3.8' # Also update `.pre-commit-config.yaml` if this change
-isort==5.10.0 ; python_version < '3.8'
-flake8>=3.9.2  # Also update `.pre-commit-config.yaml` if this change
 pytest>=3.0.2
 pytest-mock>=1.2
 pytest-cov>=2.3.1
@@ -13,3 +10,4 @@ requests-mock>=0.7.0
 jsonschema>=2.5.1
 
 black==22.8.0  # Also update `.pre-commit-config.yaml` if this changes
+ruff==0.0.261  # Also update `.pre-commit-config.yaml` if this changes

tests/notifications_python_client/test_notifications_api_client.py

@@ -331,7 +331,7 @@ def test_get_all_notifications_iterator_calls_get_notifications(notifications_cl
 def test_get_all_notifications_iterator_stops_if_empty_notification_list_returned(notifications_client, rmock):
     responses = [
         _generate_response("79f9c6ce-cd6a-4b47-a3e7-41e155f112b0", [1, 2]),
-        _generate_response("3e8f2f0a-0f2b-4d1b-8a01-761f14a281bb"),
+        _generate_response("3e8f2f0a-0f2b-4d1b-8a01-761f14a281bb", []),
     ]
 
     endpoint = "{0}/v2/notifications".format(TEST_HOST)
@@ -345,7 +345,7 @@ def test_get_all_notifications_iterator_gets_more_notifications_with_correct_id(
     responses = [
         _generate_response("79f9c6ce-cd6a-4b47-a3e7-41e155f112b0", [1, 2]),
         _generate_response("ea179232-3190-410d-b8ab-23dfecdd3157", [3, 4]),
-        _generate_response("3e8f2f0a-0f2b-4d1b-8a01-761f14a281bb"),
+        _generate_response("3e8f2f0a-0f2b-4d1b-8a01-761f14a281bb", []),
     ]
 
     endpoint = "{0}/v2/notifications".format(TEST_HOST)
@@ -411,7 +411,7 @@ def test_get_pdf_for_letter(notifications_client, rmock):
     assert rmock.called
 
 
-def _generate_response(next_link_uuid, notifications=[]):
+def _generate_response(next_link_uuid, notifications: list):
     return {
         "json": {
             "notifications": notifications,