Merge pull request #101 from communitiesuk/bau/fix-logging

Stop logging with eagerly-formatted strings / move to `ruff`
communitiesuk · Apr 25, 2024 · f9d65a0 · f9d65a0
2 parents 12c2541 + d7ae163
commit f9d65a0
Show file tree

Hide file tree

Showing 17 changed files with 84 additions and 162 deletions.
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -2,30 +2,24 @@ ci:
   autofix_prs: false
 
 repos:
--   repo: https://github.com/psf/black
-    rev: 24.2.0
-    hooks:
-    - id: black
-      language_version: python3.11
--   repo: https://github.com/PyCQA/flake8
-    rev: 7.0.0
-    hooks:
-    - id: flake8
--   repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.5.0
-    hooks:
-      - id: trailing-whitespace
-      - id: end-of-file-fixer
-      - id: check-ast
--   repo: https://github.com/pycqa/isort
-    rev: 5.13.2
-    hooks:
-      - id: isort
-        name: isort (python)
-        args: [ "--profile", "black" ]
-#-   repo: https://github.com/Riverside-Healthcare/djLint
-#    rev: v1.24.0
-#    hooks:
-#      - id: djlint-jinja
-#        types_or: ['html', 'jinja']
+- repo: https://github.com/pre-commit/pre-commit-hooks
+  rev: v4.5.0
+  hooks:
+    - id: trailing-whitespace
+    - id: end-of-file-fixer
+    - id: check-ast
+- repo: https://github.com/astral-sh/ruff-pre-commit
+  # Ruff version.
+  rev: v0.4.1
+  hooks:
+    # Run the linter.
+    - id: ruff
+      args: [ --fix ]
+    # Run the formatter.
+    - id: ruff-format
+# - repo: https://github.com/Riverside-Healthcare/djLint
+#   rev: v1.24.0
+#   hooks:
+#     - id: djlint-jinja
+#       types_or: ['html', 'jinja']
 # TODO: turn this back on and fix issues
diff --git a/app/__init__.py b/app/__init__.py
@@ -51,9 +51,7 @@ def create_app(config_class=Config):
     app.static_folder = "static/dist/"
     assets.init_app(app)
 
-    static_assets.init_assets(
-        app, auto_build=Config.AUTO_BUILD_ASSETS, static_folder="static/dist"
-    )
+    static_assets.init_assets(app, auto_build=Config.AUTO_BUILD_ASSETS, static_folder="static/dist")
 
     talisman.init_app(app, content_security_policy=csp, force_https=False)
     WTFormsHelpers(app)

diff --git a/app/main/data.py b/app/main/data.py
@@ -16,17 +16,14 @@ def get_response(hostname: str, endpoint: str, query_params: dict = None) -> Res
     :param query_params: Optional dictionary of query parameters to include in the request URL.
     :return: The requests Response object containing the response from the remote server.
     """
-    request_url = (
-        hostname
-        + endpoint
-        + ("?" + urlencode(query_params, doseq=True) if query_params else "")
-    )
+    request_url = hostname + endpoint + ("?" + urlencode(query_params, doseq=True) if query_params else "")
     response = requests.get(request_url)
     if response.status_code in [200, 404]:
         return response
 
     else:
         current_app.logger.error(
-            f"Bad response: {request_url} returned {response.status_code}"
+            "Bad response: {request_url} returned {status_code}",
+            extra=dict(request_url=request_url, status_code=response.status_code),
         )
         return abort(500)
diff --git a/app/main/download_data.py b/app/main/download_data.py
@@ -173,9 +173,7 @@ def generate_financial_years(start_date, end_date):
     max_year = end_date.year if end_date.month > 3 else end_date.year - 1
 
     # Generate the list of financial years
-    financial_years = [
-        "{}/{}".format(year, year + 1) for year in range(min_year, max_year + 1)
-    ]
+    financial_years = ["{}/{}".format(year, year + 1) for year in range(min_year, max_year + 1)]
 
     return financial_years
 
@@ -208,9 +206,7 @@ def get_returns() -> dict[str, Any]:
     if not returns_data:
         years = []
     else:
-        start_date = datetime.strptime(
-            returns_data["start_date"].split("T")[0], "%Y-%m-%d"
-        )
+        start_date = datetime.strptime(returns_data["start_date"].split("T")[0], "%Y-%m-%d")
         end_date = datetime.strptime(returns_data["end_date"].split("T")[0], "%Y-%m-%d")
         years = generate_financial_years(start_date, end_date)
 
@@ -238,9 +234,7 @@ def process_api_response(query_params: dict) -> tuple:
     Raises:
         abort(500): If an unexpected content type is received from the API.
     """
-    response = get_response(
-        Config.DATA_STORE_API_HOST, "/download", query_params=query_params
-    )
+    response = get_response(Config.DATA_STORE_API_HOST, "/download", query_params=query_params)
 
     content_type = response.headers["content-type"]
 
@@ -250,7 +244,8 @@ def process_api_response(query_params: dict) -> tuple:
         file_content = io.BytesIO(response.content)
     else:
         current_app.logger.error(
-            f"Response with unexpected content type received from API: {content_type}"
+            "Response with unexpected content type received from API: {content_type}",
+            extra=dict(content_type=content_type),
         )
         return abort(500), f"Unknown content type: {content_type}"
 

diff --git a/app/main/forms.py b/app/main/forms.py
@@ -8,18 +8,14 @@ class CookiesForm(FlaskForm):
     functional = RadioField(
         "Do you want to accept functional cookies?",
         widget=GovRadioInput(),
-        validators=[
-            InputRequired(message="Select yes if you want to accept functional cookies")
-        ],
+        validators=[InputRequired(message="Select yes if you want to accept functional cookies")],
         choices=[("no", "No"), ("yes", "Yes")],
         default="no",
     )
     analytics = RadioField(
         "Do you want to accept analytics cookies?",
         widget=GovRadioInput(),
-        validators=[
-            InputRequired(message="Select yes if you want to accept analytics cookies")
-        ],
+        validators=[InputRequired(message="Select yes if you want to accept analytics cookies")],
         choices=[("no", "No"), ("yes", "Yes")],
         default="no",
     )

diff --git a/app/main/routes.py b/app/main/routes.py
@@ -94,7 +94,8 @@ def download():
         file_format = form.file_format.data
         if file_format not in ["json", "xlsx"]:
             current_app.logger.error(
-                f"Unexpected file format requested from /download: {file_format}"
+                "Unexpected file format requested from /download: {file_format}",
+                extra=dict(file_format=file_format),
             )
             return abort(500), f"Unknown file format: {file_format}"
 
@@ -110,15 +111,11 @@ def download():
         current_datetime = datetime.now().strftime("%Y-%m-%d-%H%M%S")
 
         reporting_period_start = (
-            financial_quarter_from_mapping(quarter=from_quarter, year=from_year)
-            if to_quarter and to_year
-            else None
+            financial_quarter_from_mapping(quarter=from_quarter, year=from_year) if to_quarter and to_year else None
         )
 
         reporting_period_end = (
-            financial_quarter_to_mapping(quarter=to_quarter, year=to_year)
-            if to_quarter and to_year
-            else None
+            financial_quarter_to_mapping(quarter=to_quarter, year=to_year) if to_quarter and to_year else None
         )
 
         query_params = {"file_format": file_format}
@@ -138,12 +135,12 @@ def download():
         content_type, file_content = process_api_response(query_params)
 
         current_app.logger.info(
-            {
-                "request_type": "download",
+            "Request for download by {user_id=} with {query_params=}",
+            extra={
                 "user_id": g.account_id,
                 "email": g.user.email,
                 "query_params": query_params,
-            }
+            },
         )
         return send_file(
             file_content,
@@ -178,9 +175,7 @@ def cookies():
         response = make_response(render_template("cookies.html", form=form))
 
         # Set cookies policy for one year
-        response.set_cookie(
-            "cookies_policy", json.dumps(cookies_policy), max_age=31557600
-        )
+        response.set_cookie("cookies_policy", json.dumps(cookies_policy), max_age=31557600)
         return response
     if request.method == "GET":
         if request.cookies.get("cookies_policy"):
@@ -227,7 +222,7 @@ def http_exception(error):
     if error.code in error_templates:
         return render_template(f"{error.code}.html"), error.code
     else:
-        current_app.logger.info(f"Unhandled HTTP error {error.code} found.")
+        current_app.logger.info("Unhandled HTTP error {error_code} found.", extra=dict(error_code=error.code))
         return render_template("500.html"), error.code
 
 

diff --git a/build.py b/build.py
@@ -23,9 +23,7 @@ def build_govuk_assets(static_dist_root="app/static/dist"):
     # Checks if GovUK Frontend Assets already built
     print(DIST_PATH)
     if os.path.exists(DIST_PATH):
-        print(
-            "GovUK Frontend assets already built. If you require a rebuild manually run build.build_govuk_assets"
-        )
+        print("GovUK Frontend assets already built. If you require a rebuild manually run build.build_govuk_assets")
         return True
 
     # Download zips from GOVUK_URL

diff --git a/config/envs/default.py b/config/envs/default.py
@@ -13,9 +13,7 @@ class DefaultConfig(object):
     FSD_LOG_LEVEL = logging.INFO
     CONTACT_EMAIL = os.environ.get("CONTACT_EMAIL", "[email protected]")
     CONTACT_PHONE = os.environ.get("CONTACT_PHONE", "12345678910")
-    DEPARTMENT_NAME = os.environ.get(
-        "DEPARTMENT_NAME", "Department for Levelling Up, Housing and Communities"
-    )
+    DEPARTMENT_NAME = os.environ.get("DEPARTMENT_NAME", "Department for Levelling Up, Housing and Communities")
     DEPARTMENT_URL = os.environ.get(
         "DEPARTMENT_URL",
         "https://www.gov.uk/government/organisations/department-for-levelling-up-housing-and-communities",

diff --git a/config/envs/development.py b/config/envs/development.py
@@ -9,9 +9,7 @@
 class DevelopmentConfig(DefaultConfig):
     # RSA 256 KEYS
     if not hasattr(DefaultConfig, "RSA256_PUBLIC_KEY"):
-        _test_public_key_path = (
-            DefaultConfig.FLASK_ROOT + "/tests/keys/rsa256/public.pem"
-        )
+        _test_public_key_path = DefaultConfig.FLASK_ROOT + "/tests/keys/rsa256/public.pem"
         with open(_test_public_key_path, mode="rb") as public_key_file:
             RSA256_PUBLIC_KEY = public_key_file.read()
     FSD_LOG_LEVEL = logging.DEBUG

diff --git a/config/envs/unit_test.py b/config/envs/unit_test.py
@@ -10,9 +10,7 @@ class UnitTestConfig(DefaultConfig):
     DATA_STORE_API_HOST = "http://data-store"
     # RSA 256 KEYS
     if not hasattr(DefaultConfig, "RSA256_PUBLIC_KEY"):
-        _test_public_key_path = (
-            DefaultConfig.FLASK_ROOT + "/tests/keys/rsa256/public.pem"
-        )
+        _test_public_key_path = DefaultConfig.FLASK_ROOT + "/tests/keys/rsa256/public.pem"
         with open(_test_public_key_path, mode="rb") as public_key_file:
             RSA256_PUBLIC_KEY = public_key_file.read()
 

diff --git a/pyproject.toml b/pyproject.toml
@@ -0,0 +1,22 @@
+[tool.ruff]
+line-length = 120
+
+target-version = "py311"
+
+[tool.ruff.lint]
+select = [
+    "E",  # pycodestyle
+    "W",  # pycodestyle
+    "F",  # pyflakes
+    "I",  # isort
+    "B",  # flake8-bugbear
+    "C90",  # mccabe cyclomatic complexity
+    "G",  # flake8-logging-format
+]
+ignore = []
+exclude = [
+    "venv*",
+    ".venv*",
+    "__pycache__",
+]
+mccabe.max-complexity = 12
diff --git a/requirements-dev.in b/requirements-dev.in
@@ -1,10 +1,7 @@
 -r requirements.txt
 
 bandit==1.7.5
-black==22.12.0
 debugpy
-flake8-bugbear==23.3.23
-isort==5.12.0
 pep8-naming==0.13.3
 pip-tools==6.13.0
 pur==7.1.0
@@ -13,3 +10,4 @@ pytest
 pytest-env
 requests-mock
 pytest-mock
+ruff
diff --git a/requirements-dev.txt b/requirements-dev.txt
@@ -12,8 +12,6 @@ async-timeout==4.0.2
     # via
     #   -r requirements.txt
     #   redis
-attrs==23.1.0
-    # via flake8-bugbear
 babel==2.12.1
     # via
     #   -r requirements.txt
@@ -24,8 +22,6 @@ beautifulsoup4==4.12.2
     # via
     #   -r requirements.txt
     #   funding-service-design-utils
-black==22.12.0
-    # via -r requirements-dev.in
 blinker==1.6.2
     # via
     #   -r requirements.txt
@@ -62,7 +58,6 @@ charset-normalizer==3.1.0
 click==8.1.3
     # via
     #   -r requirements.txt
-    #   black
     #   flask
     #   pip-tools
     #   pur
@@ -95,11 +90,7 @@ docopt==0.6.2
 email-validator==2.0.0.post2
     # via -r requirements.txt
 flake8==6.0.0
-    # via
-    #   flake8-bugbear
-    #   pep8-naming
-flake8-bugbear==23.3.23
-    # via -r requirements-dev.in
+    # via pep8-naming
 flask==2.3.2
     # via
     #   -r requirements.txt
@@ -164,8 +155,6 @@ idna==3.4
     #   requests
 iniconfig==2.0.0
     # via pytest
-isort==5.12.0
-    # via -r requirements-dev.in
 itsdangerous==2.1.2
     # via
     #   -r requirements.txt
@@ -198,24 +187,18 @@ markupsafe==2.1.2
     #   wtforms
 mccabe==0.7.0
     # via flake8
-mypy-extensions==1.0.0
-    # via black
 notifications-python-client==9.0.0
     # via -r requirements.txt
 packaging==23.1
     # via
     #   build
     #   pytest
-pathspec==0.11.1
-    # via black
 pbr==5.11.1
     # via stevedore
 pep8-naming==0.13.3
     # via -r requirements-dev.in
 pip-tools==6.13.0
     # via -r requirements-dev.in
-platformdirs==3.4.0
-    # via black
 pluggy==1.0.0
     # via pytest
 pur==7.1.0
@@ -300,6 +283,8 @@ rich==12.6.0
     #   -r requirements.txt
     #   bandit
     #   funding-service-design-utils
+ruff==0.4.1
+    # via -r requirements-dev.in
 s3transfer==0.6.1
     # via
     #   -r requirements.txt

diff --git a/scripts/extract_download_logs.py b/scripts/extract_download_logs.py
@@ -77,7 +77,6 @@ def rows_to_csv(data: List[dict], field_names: List[str]) -> StringIO:
 
 
 def main(args):
-
     ENVIRONMENT = args.environment
 
     FIELD_NAMES = [

diff --git a/setup.cfg b/setup.cfg
Original file line number	Diff line number	Diff line change
Expand Up		@@ -77,7 +77,6 @@ def rows_to_csv(data: List[dict], field_names: List[str]) -> StringIO:


		def main(args):

		ENVIRONMENT = args.environment

		FIELD_NAMES = [
Expand Down