From d2fc80368dcebd6df6ebce586147965a98396023 Mon Sep 17 00:00:00 2001
From: Karel van Klink <karel.vanklink@geant.org>
Date: Fri, 24 Nov 2023 10:28:54 +0100
Subject: [PATCH] update codebase to use ruff-compatible codestyle
---
lso/__init__.py | 1 -
lso/config.py | 15 +++++++--------
lso/environment.py | 5 +++--
lso/playbook.py | 20 ++++++++++++--------
pyproject.toml | 5 +++--
test/routes/test_default.py | 3 ++-
test/routes/test_ip_trunk.py | 17 +++++++++++------
test/routes/test_router.py | 3 ++-
test/test_config.py | 9 +++++----
9 files changed, 45 insertions(+), 33 deletions(-)
diff --git a/lso/__init__.py b/lso/__init__.py
index 03ff9e5..746cd00 100644
--- a/lso/__init__.py
+++ b/lso/__init__.py
@@ -15,7 +15,6 @@ def create_app() -> FastAPI:
:return: a new flask app instance
"""
app = FastAPI()
- # app = FastAPI(dependencies=[Depends(get_query_token)])
app.add_middleware(
CORSMiddleware,
diff --git a/lso/config.py b/lso/config.py
index edf5546..240d94a 100644
--- a/lso/config.py
+++ b/lso/config.py
@@ -7,7 +7,7 @@ Config file location can also be loaded from environment variable `SETTINGS_FILE
import json
import os
-from typing import TextIO
+from pathlib import Path
import jsonschema
from pydantic import BaseModel
@@ -28,17 +28,17 @@ class Config(BaseModel):
ansible_playbooks_root_dir: str
-def load_from_file(file: TextIO) -> Config:
+def load_from_file(file: Path) -> Config:
"""Load, validate and return configuration parameters.
Input is validated against this jsonschema:
.. asjson:: lso.config.CONFIG_SCHEMA
- :param file: file-like object that produces the config file
- :return: a dict containing the parsed configuration parameters
+ :param file: :class:`Path` object that produces the config file.
+ :return: a dict containing the parsed configuration parameters.
"""
- config = json.loads(file.read())
+ config = json.loads(file.read_text())
jsonschema.validate(config, CONFIG_SCHEMA)
return Config(**config)
@@ -50,6 +50,5 @@ def load() -> Config:
:return: a dict containing the parsed configuration parameters
"""
- assert "SETTINGS_FILENAME" in os.environ, "Environment variable SETTINGS_FILENAME not set"
- with open(os.environ["SETTINGS_FILENAME"], encoding="utf-8") as file:
- return load_from_file(file)
+ assert "SETTINGS_FILENAME" in os.environ, "Environment variable SETTINGS_FILENAME not set" # noqa: S101
+ return load_from_file(Path(os.environ["SETTINGS_FILENAME"]))
diff --git a/lso/environment.py b/lso/environment.py
index 5d02d16..f8baddf 100644
--- a/lso/environment.py
+++ b/lso/environment.py
@@ -3,6 +3,7 @@
import json
import logging.config
import os
+from pathlib import Path
LOGGING_DEFAULT_CONFIG = {
"version": 1,
@@ -35,7 +36,7 @@ def setup_logging() -> None:
logging_config = LOGGING_DEFAULT_CONFIG
if "LOGGING_CONFIG" in os.environ:
filename = os.environ["LOGGING_CONFIG"]
- with open(filename, encoding="utf-8") as file:
- logging_config = json.loads(file.read())
+ config_file = Path(filename).read_text()
+ logging_config = json.loads(config_file)
logging.config.dictConfig(logging_config)
diff --git a/lso/playbook.py b/lso/playbook.py
index d04737c..5d81681 100644
--- a/lso/playbook.py
+++ b/lso/playbook.py
@@ -3,15 +3,16 @@
import enum
import json
import logging
-import os
import threading
import uuid
+from pathlib import Path
from typing import Any
import ansible_runner
import requests
import xmltodict
from dictdiffer import diff
+from fastapi import status
from pydantic import BaseModel, HttpUrl
from lso import config
@@ -48,9 +49,10 @@ class PlaybookLaunchResponse(BaseModel):
info: str = ""
-def get_playbook_path(playbook_name: str) -> str:
+def get_playbook_path(playbook_name: str) -> Path:
+ """Get the path of a playbook on the local filesystem."""
config_params = config.load()
- return os.path.join(config_params.ansible_playbooks_root_dir, playbook_name)
+ return Path(config_params.ansible_playbooks_root_dir) / playbook_name
def playbook_launch_success(job_id: str) -> PlaybookLaunchResponse:
@@ -121,7 +123,7 @@ def _process_json_output(runner: ansible_runner.Runner) -> list[dict[Any, Any]]:
before_parsed = xmltodict.parse(task_result["diff"]["before"])
after_parsed = xmltodict.parse(task_result["diff"]["after"])
# Only leave the diff in the resulting output
- task_result["diff"] = list(diff(before_parsed, after_parsed))[0]
+ task_result["diff"] = next(iter(diff(before_parsed, after_parsed)))
if bool(task_result):
# Only add the event if there are any relevant keys left.
@@ -170,13 +172,15 @@ def _run_playbook_proc(
}
request_result = requests.post(callback, json=payload, timeout=DEFAULT_REQUEST_TIMEOUT)
- assert request_result.status_code == 200, f"Callback failed: {request_result.text}"
+ if request_result.status_code != status.HTTP_200_OK:
+ msg = f"Callback failed: {request_result.text}"
+ logger.error(msg)
-def run_playbook(playbook_path: str, extra_vars: dict, inventory: str, callback: HttpUrl) -> PlaybookLaunchResponse:
+def run_playbook(playbook_path: Path, extra_vars: dict, inventory: str, callback: HttpUrl) -> PlaybookLaunchResponse:
"""Run an Ansible playbook against a specified inventory.
- :param str playbook_path: playbook to be executed.
+ :param Path playbook_path: playbook to be executed.
:param dict extra_vars: Any extra vars needed for the playbook to run.
:param [str] inventory: The inventory that the playbook is executed against.
:param :class:`HttpUrl` callback: Callback URL where the playbook should send a status update when execution is
@@ -189,7 +193,7 @@ def run_playbook(playbook_path: str, extra_vars: dict, inventory: str, callback:
target=_run_playbook_proc,
kwargs={
"job_id": job_id,
- "playbook_path": playbook_path,
+ "playbook_path": str(playbook_path),
"inventory": inventory,
"extra_vars": extra_vars,
"callback": callback,
diff --git a/pyproject.toml b/pyproject.toml
index 4be30a8..6e2a7fa 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -36,7 +36,8 @@ ignore = [
"N805",
"PLR0913",
"PLR0904",
- "PLW1514"
+ "PLW1514",
+ "S104"
]
line-length = 120
select = [
@@ -95,7 +96,7 @@ target-version = "py311"
ban-relative-imports = "all"
[tool.ruff.per-file-ignores]
-"test/*" = ["D", "S101", "PLR2004"]
+"test/*" = ["D", "S101"]
"setup.py" = ["D100"]
[tool.ruff.isort]
diff --git a/test/routes/test_default.py b/test/routes/test_default.py
index 50ec783..3d88dfb 100644
--- a/test/routes/test_default.py
+++ b/test/routes/test_default.py
@@ -2,6 +2,7 @@ from importlib import metadata
import jsonschema
import responses
+from fastapi import status
from starlette.testclient import TestClient
from lso.routes.default import API_VERSION, Version
@@ -10,7 +11,7 @@ from lso.routes.default import API_VERSION, Version
@responses.activate
def test_ip_trunk_modification(client: TestClient) -> None:
rv = client.get("/api/version/")
- assert rv.status_code == 200, rv.text
+ assert rv.status_code == status.HTTP_200_OK, rv.text
response = rv.json()
jsonschema.validate(response, Version.model_json_schema())
diff --git a/test/routes/test_ip_trunk.py b/test/routes/test_ip_trunk.py
index 9b931bb..aefb44d 100644
--- a/test/routes/test_ip_trunk.py
+++ b/test/routes/test_ip_trunk.py
@@ -6,6 +6,7 @@ import jsonschema
import pytest
import responses
from faker import Faker
+from fastapi import status
from starlette.testclient import TestClient
from lso.playbook import PlaybookLaunchResponse
@@ -166,7 +167,9 @@ def migration_object(faker: Faker) -> dict:
@responses.activate
def test_ip_trunk_provisioning(
- client: TestClient, subscription_object: dict, mocked_ansible_runner_run: Callable,
+ client: TestClient,
+ subscription_object: dict,
+ mocked_ansible_runner_run: Callable,
) -> None:
responses.post(url=TEST_CALLBACK_URL, status=200)
@@ -182,7 +185,7 @@ def test_ip_trunk_provisioning(
with patch("lso.playbook.ansible_runner.run", new=mocked_ansible_runner_run) as _:
rv = client.post("/api/ip_trunk/", json=params)
- assert rv.status_code == 200
+ assert rv.status_code == status.HTTP_200_OK
response = rv.json()
# wait a second for the run thread to finish
time.sleep(1)
@@ -195,7 +198,9 @@ def test_ip_trunk_provisioning(
@responses.activate
def test_ip_trunk_modification(
- client: TestClient, subscription_object: dict, mocked_ansible_runner_run: Callable,
+ client: TestClient,
+ subscription_object: dict,
+ mocked_ansible_runner_run: Callable,
) -> None:
responses.post(url=TEST_CALLBACK_URL, status=200)
@@ -211,7 +216,7 @@ def test_ip_trunk_modification(
with patch("lso.playbook.ansible_runner.run", new=mocked_ansible_runner_run) as _:
rv = client.put("/api/ip_trunk/", json=params)
- assert rv.status_code == 200
+ assert rv.status_code == status.HTTP_200_OK
response = rv.json()
# wait a second for the run thread to finish
time.sleep(1)
@@ -237,7 +242,7 @@ def test_ip_trunk_deletion(client: TestClient, subscription_object: dict, mocked
with patch("lso.playbook.ansible_runner.run", new=mocked_ansible_runner_run) as _:
rv = client.request(url="/api/ip_trunk/", method=responses.DELETE, json=params)
- assert rv.status_code == 200
+ assert rv.status_code == status.HTTP_200_OK
response = rv.json()
# wait a second for the run thread to finish
time.sleep(1)
@@ -270,7 +275,7 @@ def test_ip_trunk_migration(
with patch("lso.playbook.ansible_runner.run", new=mocked_ansible_runner_run) as _:
rv = client.post(url="/api/ip_trunk/migrate", json=params)
- assert rv.status_code == 200
+ assert rv.status_code == status.HTTP_200_OK
response = rv.json()
# Wait a second for the run to finish
time.sleep(1)
diff --git a/test/routes/test_router.py b/test/routes/test_router.py
index f0706b6..08f8188 100644
--- a/test/routes/test_router.py
+++ b/test/routes/test_router.py
@@ -5,6 +5,7 @@ from unittest.mock import patch
import jsonschema
import responses
from faker import Faker
+from fastapi import status
from starlette.testclient import TestClient
from lso.playbook import PlaybookLaunchResponse
@@ -47,7 +48,7 @@ def test_router_provisioning(client: TestClient, faker: Faker, mocked_ansible_ru
with patch("lso.playbook.ansible_runner.run", new=mocked_ansible_runner_run) as _:
rv = client.post("/api/router/", json=params)
- assert rv.status_code == 200
+ assert rv.status_code == status.HTTP_200_OK
response = rv.json()
# wait two seconds for the run thread to finish
time.sleep(2)
diff --git a/test/test_config.py b/test/test_config.py
index c1d54d8..00875ea 100644
--- a/test/test_config.py
+++ b/test/test_config.py
@@ -1,8 +1,9 @@
"""Set of tests that verify correct config is accepted and incorrect config is not."""
-import io
import json
import os
+import tempfile
+from pathlib import Path
import jsonschema
import pytest
@@ -29,7 +30,7 @@ def test_validate_testenv_config(data_config_filename: str) -> None:
],
)
def test_bad_config(bad_config: dict) -> None:
- with io.StringIO(json.dumps(bad_config)) as file:
- file.seek(0) # rewind file position to the beginning
+ with tempfile.NamedTemporaryFile(mode="w") as file:
+ Path(file.name).write_text(json.dumps(bad_config))
with pytest.raises(jsonschema.ValidationError):
- config.load_from_file(file)
+ config.load_from_file(Path(file.name))
--
GitLab