Second bunch of code improvements (#7)

* Add missing docstrings for return values
* Move the settings to one place
* Fix the typo in gitignore
* Fix the RET505 lint
* Provide service arguments as dictionary
* Add a doctype to HTML template
* Change page default title
* Handle various types of responses when celery is disabled

.gitignore

@@ -6,5 +6,5 @@ venv
 .vscode
 dist
 .ruff_cache
-.mypy_chache
+.mypy_cache
 __pycache__

src/tmt_web/api.py

@@ -5,9 +5,9 @@
 from fastapi import FastAPI
 from fastapi.params import Query
 from pydantic import BaseModel
-from starlette.responses import HTMLResponse
+from starlette.responses import HTMLResponse, JSONResponse, PlainTextResponse
 
-from tmt_web import service
+from tmt_web import service, settings
 from tmt_web.generators import html_generator
 
 app = FastAPI()
@@ -98,36 +98,39 @@ def find_test(
     if plan_url is None and plan_name is None and test_url is None and test_name is None:
         return "Missing arguments!"
         # TODO: forward to docs
+
+    service_args = {
+        "test_url": test_url,
+        "test_name": test_name,
+        "test_ref": test_ref,
+        "plan_url": plan_url,
+        "plan_name": plan_name,
+        "plan_ref": plan_ref,
+        "out_format": out_format,
+        "test_path": test_path,
+        "plan_path": plan_path,
+    }
+
     # Disable Celery if not needed
     if os.environ.get("USE_CELERY") == "false":
-        return service.main(
-            test_url=test_url,
-            test_name=test_name,
-            test_ref=test_ref,
-            plan_url=plan_url,
-            plan_name=plan_name,
-            plan_ref=plan_ref,
-            out_format=out_format,
-            test_path=test_path,
-            plan_path=plan_path)
-
-    r = service.main.delay(
-        test_url=test_url,
-        test_name=test_name,
-        test_ref=test_ref,
-        plan_url=plan_url,
-        plan_name=plan_name,
-        plan_ref=plan_ref,
-        out_format=out_format,
-        test_path=test_path,
-        plan_path=plan_path)
+        response_by_output = {
+            "html": HTMLResponse,
+            "json": JSONResponse,
+            "yaml": PlainTextResponse,
+        }
+
+        response = response_by_output.get(out_format, PlainTextResponse)
+        return response(service.main(**service_args))
+
+    r = service.main.delay(**service_args)
 
     # Special handling of response if the format is html
+    # TODO: Shouldn't be the "yaml" format also covered with a `PlainTextResponse`?
     if out_format == "html":
-        status_callback_url = f'{os.getenv("API_HOSTNAME")}/status/html?task-id={r.task_id}'
+        status_callback_url = f"{settings.API_HOSTNAME}/status/html?task-id={r.task_id}"
         return HTMLResponse(content=html_generator.generate_status_callback(r, status_callback_url))
-    else:
-        return _to_task_out(r)
+
+    return _to_task_out(r)
 
 
 @app.get("/status")
@@ -149,7 +152,7 @@ def status_html(task_id: Annotated[str | None,
             )
         ]) -> HTMLResponse:
     r = service.main.app.AsyncResult(task_id)
-    status_callback_url = f'{os.getenv("API_HOSTNAME")}/status/html?task-id={r.task_id}'
+    status_callback_url = f"{settings.API_HOSTNAME}/status/html?task-id={r.task_id}"
     return HTMLResponse(content=html_generator.generate_status_callback(r, status_callback_url))
 
 
@@ -158,7 +161,7 @@ def _to_task_out(r: AsyncResult) -> TaskOut:  # type: ignore [type-arg]
         id=r.task_id,
         status=r.status,
         result=r.traceback if r.failed() else r.result,
-        status_callback_url=f'{os.getenv("API_HOSTNAME")}/status?task-id={r.task_id}'
+        status_callback_url="{settings.API_HOSTNAME}/status?task-id={r.task_id}",
     )
 
 

src/tmt_web/generators/json_generator.py

@@ -35,7 +35,7 @@ def generate_test_json(test: tmt.Test, logger: tmt.Logger) -> str:
 
     :param test: Test object
     :param logger: tmt.Logger instance
-    :return:
+    :return: JSON data for a given test
     """
     data = _create_json_data(test, logger)
     logger.print("Generating the JSON file...")
@@ -48,7 +48,7 @@ def generate_plan_json(plan: tmt.Plan, logger: tmt.Logger) -> str:
 
     :param plan: Plan object
     :param logger: tmt.Logger instance
-    :return:
+    :return: JSON data for a given plan
     """
     data = _create_json_data(plan, logger)
     logger.print("Generating the JSON file...")
@@ -62,7 +62,7 @@ def generate_testplan_json(test: tmt.Test, plan: tmt.Plan, logger: tmt.Logger) -
     :param test: Test object
     :param plan: Plan object
     :param logger: tmt.Logger instance
-    :return:
+    :return: JSON data for a given test and plan
     """
     logger.print("Generating the JSON file...")
     data = {

src/tmt_web/generators/templates/_base.html.j2

@@ -1,7 +1,7 @@
-<!-- _base.html.j2 -->
-<html>
+<!DOCTYPE html>
+<html lang="en">
 <head>
-    <title>{{ title | default('HTML File') }}</title>
+    <title>{{ title | default('Untitled Document') }}</title>
     <meta charset="UTF-8">
 </head>
 <body>

src/tmt_web/generators/yaml_generator.py

@@ -14,7 +14,7 @@ def generate_test_yaml(test: tmt.Test, logger: Logger) -> str:
 
     :param test: Test object
     :param logger: tmt.Logger instance
-    :return:
+    :return: YAML data for a given test
     """
     yaml_data = tmt.utils.dict_to_yaml(_create_json_data(test, logger))
     print_success(logger)
@@ -27,7 +27,7 @@ def generate_plan_yaml(plan: tmt.Plan, logger: Logger) -> str:
 
     :param plan: Plan object
     :param logger: tmt.Logger instance
-    :return:
+    :return: YAML data for a given plan.
     """
     yaml_data = tmt.utils.dict_to_yaml(_create_json_data(plan, logger))
     print_success(logger)
@@ -41,7 +41,7 @@ def generate_testplan_yaml(test: tmt.Test, plan: tmt.Plan, logger: Logger) -> st
     :param test: Test object
     :param plan: Plan object
     :param logger: tmt.Logger instance
-    :return:
+    :return: YAML data for a given test and plan
     """
     data = {
         "test": _create_json_data(test, logger),

src/tmt_web/service.py

@@ -1,18 +1,16 @@
 import logging
-import os
 
 import tmt
 from celery.app import Celery  # type: ignore[attr-defined]
 from tmt.utils import Path  # type: ignore[attr-defined]
 
+from tmt_web import settings
 from tmt_web.generators import html_generator, json_generator, yaml_generator
 from tmt_web.utils import git_handler
 
 logger = tmt.Logger(logging.getLogger("tmt-logger"))
 
-redis_url = os.getenv("REDIS_URL", "redis://localhost:6379")
-
-app = Celery(__name__, broker=redis_url, backend=redis_url)
+app = Celery(__name__, broker=settings.REDIS_URL, backend=settings.REDIS_URL)
 
 
 def get_tree(url: str, name: str, ref: str | None, tree_path: str) -> tmt.base.Tree:
@@ -23,7 +21,7 @@ def get_tree(url: str, name: str, ref: str | None, tree_path: str) -> tmt.base.T
     :param name: Object name
     :param url: Object url
     :param tree_path: Object path
-    :return:
+    :return: returns a Tree object
     """
     logger.print(f"Cloning the repository for url: {url}")
     logger.print("Parsing the url and name...")
@@ -52,15 +50,16 @@ def process_test_request(test_url: str,
                          return_object: bool,
                          out_format: str) -> str | tmt.Test | None:
     """
-    This function processes the request for a test and returns the HTML file or the Test object.
+    This function processes the request for a test and returns the data in specified output format
+    or the Test object.
 
     :param test_url: Test url
     :param test_name: Test name
     :param test_ref: Test repo ref
     :param test_path: Test path
     :param return_object: Specify if the function should return the HTML file or the Test object
     :param out_format: Specifies output format
-    :return:
+    :return: the data in specified output format or the Test object
     """
 
     tree = get_tree(test_url, test_name, test_ref, test_path)
@@ -93,15 +92,16 @@ def process_plan_request(plan_url: str,
                          return_object: bool,
                          out_format: str) -> str | None | tmt.Plan:
     """
-    This function processes the request for a plan and returns the HTML file or the Plan object.
+    This function processes the request for a plan and returns the data in specified output format
+    or the Plan object.
 
     :param plan_url: Plan URL
     :param plan_name: Plan name
     :param plan_ref: Plan repo ref
     :param plan_path: Plan path
     :param return_object: Specify if the function should return the HTML file or the Plan object
     :param out_format: Specifies output format
-    :return:
+    :return: the data in specified output format  or the Plan object
     """
 
     tree = get_tree(plan_url, plan_name, plan_ref, plan_path)
@@ -147,7 +147,7 @@ def process_testplan_request(test_url,
     :param plan_ref: Plan repo ref
     :param plan_path: Plan path
     :param out_format: Specifies output format
-    :return:
+    :return: page data in specified output format
     """
     test = process_test_request(test_url, test_name, test_ref, test_path, False, out_format)
     if not isinstance(test, tmt.Test):

src/tmt_web/settings.py

@@ -0,0 +1,5 @@
+import os
+
+API_HOSTNAME = os.getenv("API_HOSTNAME", default="")
+REDIS_URL = os.getenv("REDIS_URL", default="redis://localhost:6379")
+CLONE_DIR_PATH = os.getenv("CLONE_DIR_PATH", default="./.repos/")

src/tmt_web/utils/git_handler.py

@@ -1,5 +1,4 @@
 import contextlib
-import os
 from shutil import rmtree
 
 from tmt import Logger
@@ -12,6 +11,8 @@
     git,
 )
 
+from tmt_web import settings
+
 
 def checkout_branch(path: Path, logger: Logger, ref: str) -> None:
     """
@@ -20,7 +21,6 @@ def checkout_branch(path: Path, logger: Logger, ref: str) -> None:
     :param ref: Name of the ref to check out
     :param path: Path to the repository
     :param logger: Instance of Logger
-    :return:
     """
     try:
         common_instance = Common(logger=logger)
@@ -39,7 +39,6 @@ def clone_repository(url: str, logger: Logger, ref: str | None = None) -> None:
     :param url: URL to the repository
     :param logger: Instance of Logger
     :param ref: Optional name of the ref to check out
-    :return:
     """
     logger.print("Cloning the repository...")
     path = get_path_to_repository(url)
@@ -80,7 +79,7 @@ def get_path_to_repository(url: str) -> Path:
     """
     repo_name = url.rstrip("/").rsplit("/", 1)[-1]
     root_dir = Path(__file__).resolve().parents[2]  # going up from tmt_web/utils/git_handler.py
-    return root_dir / os.getenv("CLONE_DIR_PATH", "./.repos/") / repo_name
+    return root_dir / settings.CLONE_DIR_PATH / repo_name
 
 
 def check_if_repository_exists(url: str) -> bool:
@@ -98,11 +97,10 @@ def clear_tmp_dir(logger: Logger) -> None:
     Clears the .tmp directory.
 
     :param logger: Instance of Logger
-    :return:
     """
     logger.print("Clearing the .tmp directory...")
     root_dir = Path(__file__).resolve().parents[2]  # going up from tmt_web/utils/git_handler.py
-    path = root_dir / os.getenv("CLONE_DIR_PATH", "./.repos/")
+    path = root_dir / settings.CLONE_DIR_PATH
     try:
         rmtree(path)
     except Exception as e: