diff --git a/lso/__init__.py b/lso/__init__.py index 03ff9e5912c06932103808428157e7774dcd4853..746cd003a67a9418f9493ac989e391178f93e896 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 edf5546fe2020a81c43d37bbffd68ce2580d71b1..240d94a1404d75c1b83528d5813d2cbacd8e83fe 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 5d02d1689d133750034095cd3781fea997064a50..f8baddfcc65d31f95e5185d29a2119d6b88c3af0 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 d04737cbbd5041a3fda24c88fc686927aaf8813d..5d816817bb77e2a6ffeac99e830e1ecb166e308c 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 4be30a8e9a06f11a435b2b94df7eede70816d420..6e2a7fa7d01a01cd2afb2110587b506524be1107 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 50ec783d6af75ce9d5be7c2e4ef4a319c6928ea3..3d88dfb3c0c1e41db4eca2306d0ed8c9f15ab7bf 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 9b931bba283e6033817d0acdf07b18481d7b7598..aefb44d3ff12b698ee83f1ccb729b975cc10e218 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 f0706b66c0cef72bd942e6bd5b4ae08cbb08cbc8..08f81881eaf7221b45d51fe34d30801e4c82fc03 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 c1d54d8a34be8639386a6ade376564b9961131fb..00875eae7d8bd4281ab470e7faa251d9012506da 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))