From d89a13eeeb9ebf9f65f491daca00f3d020ccf452 Mon Sep 17 00:00:00 2001 From: "Felipe N. Schuch" Date: Fri, 27 Sep 2024 16:52:47 -0300 Subject: [PATCH] solve feedback on acceptance (#613) migrated docstrings from sphinx to google style renamed function garbage_collect to garbage_collector Fixed spelling errors in the docstrings --- .../jobbergate_agent/jobbergate/submit.py | 9 +- jobbergate-agent/jobbergate_agent/tasks.py | 2 +- .../jobbergate_agent/utils/exception.py | 60 ++++++------ .../jobbergate_agent/utils/logging.py | 5 +- .../jobbergate_api/apps/file_validation.py | 23 +++-- .../jobbergate_api/apps/garbage_collector.py | 2 +- .../apps/job_script_templates/routers.py | 4 +- .../apps/job_scripts/routers.py | 4 +- .../apps/job_submissions/routers.py | 2 +- .../jobbergate_api/apps/services.py | 4 +- .../tests/apps/test_garbage_collector.py | 4 +- jobbergate-cli/jobbergate_cli/logging.py | 2 +- jobbergate-cli/jobbergate_cli/main.py | 3 +- jobbergate-cli/jobbergate_cli/render.py | 68 +++++++------ jobbergate-cli/jobbergate_cli/requests.py | 31 +++--- .../subapps/applications/questions.py | 95 +++++++++++-------- .../subapps/applications/tools.py | 66 ++++++++----- .../subapps/job_scripts/tools.py | 89 +++++++++-------- .../subapps/job_submissions/tools.py | 28 +++--- jobbergate-core/jobbergate_core/auth/token.py | 2 +- .../docs/source/developer_guide/dev_tools.md | 2 +- .../developer_guide/integration_testing.md | 2 +- .../developer_guide/template_workflows.md | 7 +- 23 files changed, 287 insertions(+), 227 deletions(-) diff --git a/jobbergate-agent/jobbergate_agent/jobbergate/submit.py b/jobbergate-agent/jobbergate_agent/jobbergate/submit.py index 296f7f313..e89c5ac43 100644 --- a/jobbergate-agent/jobbergate_agent/jobbergate/submit.py +++ b/jobbergate-agent/jobbergate_agent/jobbergate/submit.py @@ -191,8 +191,11 @@ async def submit_job_script( """ Submit a Job Script to slurm via the sbatch command. - :param: pending_job_submission: A job_submission with fields needed to submit. - :returns: The ``slurm_job_id`` for the submitted job + Args: + pending_job_submission: A job_submission with fields needed to submit. + + Returns: + The ``slurm_job_id`` for the submitted job """ logger.debug(f"Submitting {pending_job_submission}") @@ -260,8 +263,6 @@ async def _reject_handler(params: DoExceptParams) -> None: async def submit_pending_jobs() -> None: """ Submit all pending jobs and update them with ``SUBMITTED`` status and slurm_job_id. - - :returns: The ``slurm_job_id`` for the submitted job """ logger.debug("Started submitting pending jobs...") diff --git a/jobbergate-agent/jobbergate_agent/tasks.py b/jobbergate-agent/jobbergate_agent/tasks.py index 9b5b7d8ab..0ff3adcff 100644 --- a/jobbergate-agent/jobbergate_agent/tasks.py +++ b/jobbergate-agent/jobbergate_agent/tasks.py @@ -66,7 +66,7 @@ async def trigger_garbage_collections(interval_between_calls: int = 60) -> None: def garbage_collection_task(scheduler: BaseScheduler) -> Union[Job, None]: """ - Schedule a task to perform garbage collection every dat at. + Schedule a task to perform garbage collection every dat at a specified time. """ if SETTINGS.TASK_GARBAGE_COLLECTION_HOUR is None: return None diff --git a/jobbergate-agent/jobbergate_agent/utils/exception.py b/jobbergate-agent/jobbergate_agent/utils/exception.py index acc0b83af..96fc2c35d 100644 --- a/jobbergate-agent/jobbergate_agent/utils/exception.py +++ b/jobbergate-agent/jobbergate_agent/utils/exception.py @@ -51,37 +51,35 @@ async def handle_errors_async( Async context manager that will intercept exceptions and repackage them with a message attached. Example: - - .. code-block:: python - - with handle_errors("It didn't work"): - some_code_that_might_raise_an_exception() - - :param: message: The message to attach to the raised exception. - :param: raise_exc_class: The exception type to raise with the constructed message - if an exception is caught in the managed context. - - Defaults to Exception. - - If ``None`` is passed, no new exception will be raised and only the - ``do_except``, ``do_else``, and ``do_finally`` - functions will be called. - :param: raise_args: Additional positional args (after the constructed message) that will - passed when raising an instance of the ``raise_exc_class``. - :param: raise_kwargs: Keyword args that will be passed when raising an instance of the - ``raise_exc_class``. - :param: handle_exc_class: Limits the class of exceptions that will be intercepted - Any other exception types will not be caught and re-packaged. - Defaults to Exception (will handle all exceptions). May also be - provided as a tuple of multiple exception types to handle. - :param: do_finally: A function that should always be called at the end of the block. - Should take no parameters. - :param: do_except: A function that should be called only if there was an exception. - Must accept one parameter that is an instance of the - ``DoExceptParams`` dataclass. Note that the ``do_except`` - method is passed the *original exception*. - :param: do_else: A function that should be called only if there were no - exceptions encountered. + ``` python + with handle_errors("It didn't work"): + some_code_that_might_raise_an_exception() + ``` + + Args: + message: The message to attach to the raised exception. + raise_exc_class: The exception type to raise with the constructed message + if an exception is caught in the managed context. + + Defaults to Exception. + + If ``None`` is passed, no new exception will be raised and only the + ``do_except``, ``do_else``, and ``do_finally`` + functions will be called. + raise_args: Additional positional args (after the constructed message) that will + be passed when raising an instance of the ``raise_exc_class``. + raise_kwargs: Keyword args that will be passed when raising an instance of the ``raise_exc_class``. + handle_exc_class: Limits the class of exceptions that will be intercepted. + Any other exception types will not be caught and re-packaged. + Defaults to Exception (will handle all exceptions). May also be + provided as a tuple of multiple exception types to handle. + do_finally: A function that should always be called at the end of the block. Should take no parameters. + do_except: A function that should be called only if there was an exception. + Must accept one parameter that is an instance of the + ``DoExceptParams`` dataclass. Note that the ``do_except`` + method is passed the *original exception*. + do_else: A function that should be called only if there were no + exceptions encountered. """ try: yield diff --git a/jobbergate-agent/jobbergate_agent/utils/logging.py b/jobbergate-agent/jobbergate_agent/utils/logging.py index aa23a5aa6..134c8fe2c 100644 --- a/jobbergate-agent/jobbergate_agent/utils/logging.py +++ b/jobbergate-agent/jobbergate_agent/utils/logging.py @@ -12,8 +12,9 @@ def log_error(params: DoExceptParams): Provide a utility function to log a Buzz-based exception and the stack-trace of the error's context. - :param: params: A DoExceptParams instance containing the original exception, a - message describing it, and the stack trace of the error. + Args: + params: A DoExceptParams instance containing the original exception, a + message describing it, and the stack trace of the error. """ logger.error( "\n".join( diff --git a/jobbergate-api/jobbergate_api/apps/file_validation.py b/jobbergate-api/jobbergate_api/apps/file_validation.py index 96eeb80f9..df2b654f1 100644 --- a/jobbergate-api/jobbergate_api/apps/file_validation.py +++ b/jobbergate-api/jobbergate_api/apps/file_validation.py @@ -50,8 +50,6 @@ def register_syntax_validator(*file_extensions: str): the file extensions that are provided as arguments. Raise ValueError if the provided file extensions do not start with a dot. - - :return ValidationEquation: The decorated function. """ def decorator(validator): @@ -77,8 +75,11 @@ def is_valid_python_file(source_code: Union[str, bytes]) -> bool: """ Check if a given Python source code is valid by parsing it into an AST node. - :param Union[str, bytes] source_code: Python source code. - :return bool: Boolean indicating if the source code is valid or not. + Args: + source_code: Python source code. + + Returns: + Boolean indicating if the source code is valid or not. """ try: ast_parse(source_code) @@ -92,8 +93,11 @@ def is_valid_yaml_file(yaml_file: Union[str, bytes]) -> bool: """ Check if a given YAML file is valid by parsing it with yaml.safe_load. - :param Union[str, bytes] yaml_file: YAML file. - :return bool: Boolean indicating if the file is valid or not. + Args: + yaml_file: YAML file. + + Returns: + Boolean indicating if the file is valid or not. """ try: yaml_safe_load(yaml_file) @@ -107,8 +111,11 @@ def is_valid_jinja2_template(template: Union[str, bytes]) -> bool: """ Check if a given jinja2 template is valid by creating a Template object and trying to render it. - :param str template: Jinja2 template. - :return bool: Boolean indicating if the template is valid or not. + Args: + template: Jinja2 template. + + Returns: + Boolean indicating if the template is valid or not. """ if isinstance(template, bytes): _template = template.decode("utf-8") diff --git a/jobbergate-api/jobbergate_api/apps/garbage_collector.py b/jobbergate-api/jobbergate_api/apps/garbage_collector.py index e03b29b34..4ac188057 100644 --- a/jobbergate-api/jobbergate_api/apps/garbage_collector.py +++ b/jobbergate-api/jobbergate_api/apps/garbage_collector.py @@ -47,7 +47,7 @@ async def delete_file(file: str) -> None: await asyncio.gather(*tasks) -async def garbage_collect(session, bucket, list_of_tables, background_tasks: BackgroundTasks) -> None: +async def garbage_collector(session, bucket, list_of_tables, background_tasks: BackgroundTasks) -> None: """Delete unused files from jobbergate's file storage.""" for table in list_of_tables: files_to_delete = await get_files_to_delete(session, table, bucket) diff --git a/jobbergate-api/jobbergate_api/apps/job_script_templates/routers.py b/jobbergate-api/jobbergate_api/apps/job_script_templates/routers.py index 2cf0bc21b..5c4778753 100644 --- a/jobbergate-api/jobbergate_api/apps/job_script_templates/routers.py +++ b/jobbergate-api/jobbergate_api/apps/job_script_templates/routers.py @@ -19,7 +19,7 @@ from jobbergate_api.apps.constants import FileType from jobbergate_api.apps.dependencies import SecureService, secure_services -from jobbergate_api.apps.garbage_collector import garbage_collect +from jobbergate_api.apps.garbage_collector import garbage_collector from jobbergate_api.apps.job_script_templates.constants import WORKFLOW_FILE_NAME from jobbergate_api.apps.job_script_templates.models import JobScriptTemplateFile, WorkflowFile from jobbergate_api.apps.job_script_templates.schemas import ( @@ -419,7 +419,7 @@ async def job_script_template_garbage_collector( """Delete all unused files from jobbergate templates on the file storage.""" logger.info("Starting garbage collection from jobbergate file storage") background_tasks.add_task( - garbage_collect, + garbage_collector, secure_services.session, secure_services.bucket, [JobScriptTemplateFile, WorkflowFile], diff --git a/jobbergate-api/jobbergate_api/apps/job_scripts/routers.py b/jobbergate-api/jobbergate_api/apps/job_scripts/routers.py index e5295b480..41dd233f6 100644 --- a/jobbergate-api/jobbergate_api/apps/job_scripts/routers.py +++ b/jobbergate-api/jobbergate_api/apps/job_scripts/routers.py @@ -11,7 +11,7 @@ from jobbergate_api.apps.constants import FileType from jobbergate_api.apps.dependencies import SecureService, secure_services -from jobbergate_api.apps.garbage_collector import garbage_collect +from jobbergate_api.apps.garbage_collector import garbage_collector from jobbergate_api.apps.job_script_templates.models import JobScriptTemplate from jobbergate_api.apps.job_script_templates.tools import coerce_id_or_identifier from jobbergate_api.apps.job_scripts.models import JobScriptFile @@ -390,7 +390,7 @@ def job_script_garbage_collector( """Delete all unused files from job scripts on the file storage.""" logger.info("Starting garbage collection from jobbergate file storage") background_tasks.add_task( - garbage_collect, + garbage_collector, secure_services.session, secure_services.bucket, [JobScriptFile], diff --git a/jobbergate-api/jobbergate_api/apps/job_submissions/routers.py b/jobbergate-api/jobbergate_api/apps/job_submissions/routers.py index aca225f4f..45e06534f 100644 --- a/jobbergate-api/jobbergate_api/apps/job_submissions/routers.py +++ b/jobbergate-api/jobbergate_api/apps/job_submissions/routers.py @@ -98,7 +98,7 @@ async def job_submission_get( secure_services(Permissions.ADMIN, Permissions.JOB_SUBMISSIONS_READ, commit=False) ), ): - """Return the job_submission given it's id.""" + """Return the job_submission given its id.""" logger.debug(f"Getting job submission {id=}") return await secure_services.crud.job_submission.get(id) diff --git a/jobbergate-api/jobbergate_api/apps/services.py b/jobbergate-api/jobbergate_api/apps/services.py index 89366a2be..8baca9b2d 100644 --- a/jobbergate-api/jobbergate_api/apps/services.py +++ b/jobbergate-api/jobbergate_api/apps/services.py @@ -447,7 +447,7 @@ def __tablename__(self) -> str: class FileService(DatabaseBoundService, BucketBoundService, Generic[FileModel]): """ - Proide a service that can perform various file management operations using a supplied ORM model type. + Provide a service that can perform various file management operations using a supplied ORM model type. """ model_type: type[FileModel] @@ -462,7 +462,7 @@ def __init__(self, model_type: type[FileModel]): async def get(self, parent_id: int, filename: str) -> FileModel: """ - Get a single instances by its parent id and filename (primary keys). + Get a single instance by its parent id and filename (primary keys). Requires that one and only one result is found. """ diff --git a/jobbergate-api/tests/apps/test_garbage_collector.py b/jobbergate-api/tests/apps/test_garbage_collector.py index 8683a3f79..206956c29 100644 --- a/jobbergate-api/tests/apps/test_garbage_collector.py +++ b/jobbergate-api/tests/apps/test_garbage_collector.py @@ -5,7 +5,7 @@ from jobbergate_api.apps.garbage_collector import ( delete_files_from_bucket, - garbage_collect, + garbage_collector, get_files_to_delete, get_set_of_files_from_bucket, get_set_of_files_from_database, @@ -108,7 +108,7 @@ async def test_garbage_collect(synth_session, synth_bucket, insert_file): await synth_session.delete(file2) bg_tasks = BackgroundTasks() - await garbage_collect(synth_session, synth_bucket, [JobScriptTemplateFile], bg_tasks) + await garbage_collector(synth_session, synth_bucket, [JobScriptTemplateFile], bg_tasks) await bg_tasks() bucket_files = await get_set_of_files_from_bucket(synth_bucket, JobScriptTemplateFile) diff --git a/jobbergate-cli/jobbergate_cli/logging.py b/jobbergate-cli/jobbergate_cli/logging.py index c62f7b156..0f9484795 100644 --- a/jobbergate-cli/jobbergate_cli/logging.py +++ b/jobbergate-cli/jobbergate_cli/logging.py @@ -14,7 +14,7 @@ def init_logs(verbose=False): """ Initialize logging. - If JOBBERGATE_LOG_PATH is set in the config, add a rotatating file log handler. + If JOBBERGATE_LOG_PATH is set in the config, add a rotating file log handler. Logs will be retained for 1 week. If verbose is supplied, add a stdout handler at the DEBUG level. diff --git a/jobbergate-cli/jobbergate_cli/main.py b/jobbergate-cli/jobbergate_cli/main.py index 121ffd486..3974b48a4 100644 --- a/jobbergate-cli/jobbergate_cli/main.py +++ b/jobbergate-cli/jobbergate_cli/main.py @@ -3,6 +3,7 @@ """ import sys +from typing import Any import httpx import importlib_metadata @@ -168,7 +169,7 @@ def show_token( print(token_text) else: subject = f"{token.label.title()} Token" - kwargs = dict(subject=subject, indent=False) + kwargs: dict[str, Any] = dict(subject=subject, indent=False) if on_clipboard: kwargs["footer"] = "The output was copied to your clipboard" diff --git a/jobbergate-cli/jobbergate_cli/render.py b/jobbergate-cli/jobbergate_cli/render.py index fa4be2a3d..33fde8c32 100644 --- a/jobbergate-cli/jobbergate_cli/render.py +++ b/jobbergate-cli/jobbergate_cli/render.py @@ -3,7 +3,7 @@ """ import json -from typing import Any, Callable, Dict, List, Optional, Union, cast +from typing import Any, Callable, Dict, List, Optional, cast import pydantic from rich import print_json @@ -67,17 +67,20 @@ def map_style(self, column: str) -> Dict[str, Any]: ) -def terminal_message(message, subject=None, color="green", footer=None, indent=True): +def terminal_message( + message: str, subject: str | None = None, color: str = "green", footer: str | None = None, indent: bool = True +): """ Print a nicely formatted message as output to the user using a ``rich`` ``Panel``. - :param: message: The message to print out - :param: subject: An optional subject line to add in the header of the ``Panel`` - :param: color: An optional color to style the ``subject`` header with - :param: footer: An optional message to display in the footer of the ``Panel`` - :param: indent: Adds padding to the left of the message + Args: + message: The message to print out. + subject: An optional subject line to add in the header of the ``Panel``. + color: An optional color to style the ``subject`` header with. Defaults to "green". + footer: An optional message to display in the footer of the ``Panel``. + indent: Adds padding to the left of the message. Defaults to True. """ - panel_kwargs = dict(padding=1) + panel_kwargs: dict[str, Any] = dict(padding=1) if subject is not None: panel_kwargs["title"] = f"[{color}]{subject}" if footer is not None: @@ -104,18 +107,19 @@ def render_json(data: Any): def render_list_results( ctx: JobbergateContext, envelope: ListResponseEnvelope, - style_mapper: Optional[StyleMapper] = None, - hidden_fields: Optional[List[str]] = None, + style_mapper: StyleMapper | None = None, + hidden_fields: list[str] | None = None, title: str = "Results List", ): """ Render a list of result data items in a ``rich`` ``Table``. - :param: ctx: The JobbergateContext. This is needed to detect if ``full`` or ``raw`` output is needed - :param: envelope: A ListResponseEnvelope containing the data items - :param: style_mapper: The style mapper that should be used to apply styles to the columns of the table - :param: hidden_fields: Columns that should (if not using ``full`` mode) be hidden in the ``Table`` output - :param: title: The title header to include above the ``Table`` output + Args: + ctx: The JobbergateContext. This is needed to detect if ``full`` or ``raw`` output is needed. + envelope: A ListResponseEnvelope containing the data items. + style_mapper: The style mapper that should be used to apply styles to the columns of the table. + hidden_fields: Columns that should (if not using ``full`` mode) be hidden in the ``Table`` output. + title: The title header to include above the ``Table`` output. """ if ctx.raw_output: render_json(envelope.model_dump(mode="json")["items"]) @@ -145,16 +149,17 @@ def render_list_results( def render_dict( - data: Dict[str, Any], + data: dict[str, Any], title: str = "Data", - hidden_fields: Optional[List[str]] = None, + hidden_fields: list[str] | None = None, ): """ - Render a dictionary in a ``rich`` ``Table`` That shows the key and value of each item. + Render a dictionary in a ``rich`` ``Table`` that shows the key and value of each item. - :param: data: The dictionary to render - :param: title: The title header to include above the ``Table`` output - :param: hidden_fields: Keys that should be hidden in the ``Table`` output + Args: + data: The dictionary to render. + title: The title header to include above the ``Table`` output. + hidden_fields: Keys that should be hidden in the ``Table`` output. """ if hidden_fields is None: hidden_fields = [] @@ -175,19 +180,20 @@ def render_dict( def render_single_result( ctx: JobbergateContext, - result: Union[Dict[str, Any], pydantic.BaseModel], - hidden_fields: Optional[List[str]] = None, + result: dict[str, Any] | pydantic.BaseModel, + hidden_fields: list[str] | None = None, title: str = "Result", - value_mappers: Optional[Dict[str, Callable[[Any], Any]]] = None, + value_mappers: dict[str, Callable[[Any], Any]] | None = None, ): """ - Render a single data item in a ``rich`` ``Table. - - :param: ctx: The JobbergateContext. This is needed to detect if ``full` or ``raw`` output is needed - :param: result: The data item to display. May be a dict or a pydantic model. - :param: hidden_fields: Rows that should (if not using ``full`` mode) be hidden in the ``Table`` output - :param: title: The title header to include above the ``Tale`` output - :param: value_mappers: Mapping functions to change fields before rendering + Render a single data item in a ``rich`` ``Table``. + + Args: + ctx: The JobbergateContext. This is needed to detect if ``full`` or ``raw`` output is needed. + result: The data item to display. May be a dict or a pydantic model. + hidden_fields: Rows that should (if not using ``full`` mode) be hidden in the ``Table`` output. + title: The title header to include above the ``Table`` output. + value_mappers: Mapping functions to change fields before rendering. """ if isinstance(result, pydantic.BaseModel): result_model = cast(pydantic.BaseModel, result) diff --git a/jobbergate-cli/jobbergate_cli/requests.py b/jobbergate-cli/jobbergate_cli/requests.py index 120136b9f..f6995040d 100644 --- a/jobbergate-cli/jobbergate_cli/requests.py +++ b/jobbergate-cli/jobbergate_cli/requests.py @@ -5,7 +5,7 @@ from __future__ import annotations from pathlib import Path -from typing import Any, Type, TypeVar, Union +from typing import Any, Type, TypeVar import httpx import pydantic @@ -133,21 +133,26 @@ def make_request( request_model: pydantic.BaseModel | None = None, save_to_file: Path | None = None, **request_kwargs: Any, -) -> Union[ResponseModel, dict, int]: +) -> ResponseModel | dict | int: """ Make a request against the Jobbergate API. - :param: client: The Httpx client to use for the request - :param: url_path: The path to add to the base url of the client where the request should be sent - :param: method: The REST method to use for the request (GET, PUT, UPDATE, POST, DELETE, etc) - :param: expected_status: The status code to expect on the response. If it is not received, raise an Abort - :param: expect_response: Indicates if response data (JSON) is expected from the API endpoint - :param: abort_message: The message to show the user if there is a problem and the app must be aborted - :param: abort_subject: The subject to use in Abort output to the user - :param: support: If true, add a message to the output instructing the user to seek help - :param: response_model_cls: If supplied, serialize the response data into this Pydantic model class - :param: request_model: Use a pydantic model instance as the data body for the request - :param: request_kwargs: Any additional keyword arguments that need to be passed on to the client + Args: + client: The Httpx client to use for the request. + url_path: The path to add to the base url of the client where the request should be sent. + method: The REST method to use for the request (GET, PUT, UPDATE, POST, DELETE, etc). + expected_status: The status code to expect on the response. If it is not received, raise an Abort. + expect_response: Indicates if response data (JSON) is expected from the API endpoint. + abort_message: The message to show the user if there is a problem and the app must be aborted. + abort_subject: The subject to use in Abort output to the user. + support: If true, add a message to the output instructing the user to seek help. + response_model_cls: If supplied, serialize the response data into this Pydantic model class. + request_model: Use a pydantic model instance as the data body for the request. + save_to_file: If supplied, save the response data to this file. + request_kwargs: Any additional keyword arguments to pass to the request. + + Returns: + The response from the API, either as a Pydantic model, a dictionary, or an integer status code. """ if request_model is not None: diff --git a/jobbergate-cli/jobbergate_cli/subapps/applications/questions.py b/jobbergate-cli/jobbergate_cli/subapps/applications/questions.py index 469cfc3e7..2dc3d0b87 100644 --- a/jobbergate-cli/jobbergate_cli/subapps/applications/questions.py +++ b/jobbergate-cli/jobbergate_cli/subapps/applications/questions.py @@ -12,7 +12,7 @@ from functools import partial from itertools import chain -from typing import Any, Callable, Dict, Optional, Type, TypeVar +from typing import Any, Callable, Dict, Type, TypeVar import inquirer import inquirer.errors @@ -36,17 +36,18 @@ def __init__( variablename: str, message: str, ignore: bool = False, - default: Optional[Any] = None, + default: Any | None = None, inquirer_type: Type[TInquirerType] = inquirer.Text, ): """ Initialize the Question. - :param: variablename: The key in the config dictionary that this question will set - :param: message: The message to show the user that describes what the question is gathering - :param: ignore: If true, do not ask the question and just use the default value instead - :param: default: The default value for the variablename in the answers dict - :param: inquirer_type: The ``inquirer`` question type that this ``QuestionBase`` wraps + Args: + variablename: The key in the config dictionary that this question will set. + message: The message to show the user that describes what the question is gathering. + ignore: If true, do not ask the question and just use the default value instead. + default: The default value for the variablename in the answers dict. + inquirer_type: The ``inquirer`` question type that this ``QuestionBase`` wraps. """ self.variablename = variablename self.default = default @@ -61,7 +62,8 @@ def make_prompts(self, **override_kwargs): """ Create ``inquirer`` prompts from this instance of ``QuestionBase``. - :param: override_kwargs: A collection of keyword arguments to override in intializing the ``inquirer`` question + Args: + override_kwargs: A collection of keyword arguments to override in initializing the ``inquirer`` question. """ final_kwargs = { **self.inquirer_kwargs, @@ -85,17 +87,18 @@ def __init__( self, variablename: str, message: str, - minval: Optional[int] = None, - maxval: Optional[int] = None, + minval: int | None = None, + maxval: int | None = None, **kwargs, ): """ Initialize the Integer question. - :param: variablename: The key in the config dictionary that this question will set - :param: message: The message to show the user that describes what the question is gathering - :param: minval: The minimum value the integer may be set to. If not specified, use negative infinity. - :param: minval: The maximum value the integer may be set to. If not specified, use infinity. + Args: + variablename: The key in the config dictionary that this question will set. + message: The message to show the user that describes what the question is gathering. + minval: The minimum value the integer may be set to. If not specified, use negative infinity. + maxval: The maximum value the integer may be set to. If not specified, use infinity. """ super().__init__(variablename, message, **kwargs) self.minval = minval @@ -134,9 +137,10 @@ def __init__(self, variablename: str, message: str, choices: list, **kwargs): """ Initialize the List question. - :param: variablename: The key in the config dictionary that this question will set - :param: message: The message to show the user that describes what the question is gathering - :param: choices: A list of the possible values from which the Question will allow the user to select one + Args: + variablename: The key in the config dictionary that this question will set. + message: The message to show the user that describes what the question is gathering. + choices: A list of the possible values from which the Question will allow the user to select one. """ super().__init__(variablename, message, inquirer_type=inquirer.List, **kwargs) self.inquirer_kwargs.update(choices=choices) @@ -144,18 +148,20 @@ def __init__(self, variablename: str, message: str, choices: list, **kwargs): class Directory(QuestionBase): """ - Asks for a directory name. If `exists` is `True` it checks if path exists and is a directory. + Asks for a directory name. If `exists` is `True`, it checks if the path exists and is a directory. - :param exists: Checks if given directory exists + Args: + exists: Checks if the given directory exists. """ - def __init__(self, variablename: str, message: str, exists: Optional[bool] = None, **kwargs): + def __init__(self, variablename: str, message: str, exists: bool | None = None, **kwargs): """ Initialize the Directory question. - :param: variablename: The key in the config dictionary that this question will set - :param: message: The message to show the user that describes what the question is gathering - :param: exists: If True, ensure that the directory exists on the system + Args: + variablename: The key in the config dictionary that this question will set. + message: The message to show the user that describes what the question is gathering. + exists: If True, ensure that the directory exists on the system. """ super().__init__(variablename, message, inquirer_type=inquirer.Path, **kwargs) @@ -169,13 +175,14 @@ class File(QuestionBase): Asks for a file name. """ - def __init__(self, variablename: str, message: str, exists: Optional[bool] = None, **kwargs): + def __init__(self, variablename: str, message: str, exists: bool | None = None, **kwargs): """ Initialize the File question. - :param: variablename: The key in the config dictionary that this question will set - :param: message: The message to show the user that describes what the question is gathering - :param: exists: If True, ensure that the file path exists on the system + Args: + variablename: The key in the config dictionary that this question will set. + message: The message to show the user that describes what the question is gathering. + exists: If True, ensure that the file path exists on the system. """ super().__init__(variablename, message, inquirer_type=inquirer.Path, **kwargs) self.inquirer_kwargs.update(path_type=inquirer.Path.FILE) @@ -192,9 +199,10 @@ def __init__(self, variablename: str, message: str, choices: list, **kwargs): """ Initialize the Checkbox question. - :param: variablename: The key in the config dictionary that this question will set - :param: message: The message to show the user that describes what the question is gathering - :param: choices: A list of the possible values from which the Question will allow the user to select many + Args: + variablename: The key in the config dictionary that this question will set. + message: The message to show the user that describes what the question is gathering. + choices: A list of the possible values from which the Question will allow the user to select many. """ super().__init__( @@ -215,8 +223,9 @@ def __init__(self, variablename: str, message: str, **kwargs): """ Initialize the Confirm question. - :param: variablename: The key in the config dictionary that this question will set - :param: message: The message to show the user that describes what the question is gathering + Args: + variablename: The key in the config dictionary that this question will set. + message: The message to show the user that describes what the question is gathering. """ super().__init__(variablename, message, inquirer_type=inquirer.Confirm, **kwargs) @@ -237,21 +246,23 @@ def __init__( """ Initialize the Checkbox question. - :param: variablename: The key in the config dictionary that this question will set - :param: message: The message to show the user that describes what the question is gathering - :param whentrue: List of questions to ask if user answers 'true' on this question - :param whentrue: List of questions to show if user answers 'false' on this question + Args: + variablename: The key in the config dictionary that this question will set. + message: The message to show the user that describes what the question is gathering. + whentrue: List of questions to ask if the user answers 'true' on this question. + whenfalse: List of questions to show if the user answers 'false' on this question. """ super().__init__(variablename, message, **kwargs) self.whentrue_child = whentrue or list() self.whenfalse_child = whenfalse or list() - def ignore_child(self, child: QuestionBase, answers: Dict[str, Any]) -> bool: + def ignore_child(self, child: QuestionBase, answers: dict[str, Any]) -> bool: """ Dynamically check if a child question should be ignored based on the questions that have already been answered. - :param: child: The child question that might be ignored - :param: answers: Answer values to previously asked questions + Args: + child: The child question that might be ignored. + answers: Answer values to previously asked questions. """ my_answer = answers.get(self.variablename) Abort.require_condition( @@ -276,7 +287,8 @@ def make_prompts(self, **override_kwargs): """ Create ``inquirer`` prompts from this instance of ``BooleanList`` and for all its child questions. - :param: override_kwargs: A collection of keyword arguments to override in the base ``make_prompts`` method + Args: + override_kwargs: A collection of keyword arguments to override in the base ``make_prompts`` method. """ retval = super().make_prompts(**override_kwargs) @@ -294,7 +306,8 @@ def __init__(self, variablename: str, **kwargs): """ Initialize the Const "question". - :param: variablename: The key in the config dictionary that this question will set + Args: + variablename: The key in the config dictionary that this question will set. """ super().__init__(variablename, "", **kwargs) diff --git a/jobbergate-cli/jobbergate_cli/subapps/applications/tools.py b/jobbergate-cli/jobbergate_cli/subapps/applications/tools.py index 928c7f0b4..25994acd3 100644 --- a/jobbergate-cli/jobbergate_cli/subapps/applications/tools.py +++ b/jobbergate-cli/jobbergate_cli/subapps/applications/tools.py @@ -48,8 +48,11 @@ def fetch_application_data_locally( """ Retrieve an application from a local directory. - :param: application_path: The directory containing the application files - :returns: A LocalApplication instance containing the application data + Args: + application_path: The directory containing the application files. + + Returns: + A LocalApplication instance containing the application data. """ Abort.require_condition(application_path.is_dir(), f"Application directory {application_path} does not exist") @@ -107,12 +110,15 @@ def fetch_application_data( identifier: Optional[str] = None, ) -> ApplicationResponse: """ - Retrieve an application from the API by ``id`` or ``identifier``. + Retrieve an application from the API by id or identifier. + + Args: + jg_ctx: The JobbergateContext. Needed to access the Httpx client with which to make API calls. + id: The id of the application to fetch. + identifier: If supplied, look for an application instance with the provided identifier. - :param: jg_ctx: The JobbergateContext. Needed to access the Httpx client with which to make API calls - :param: id: The id of the application to fetch - :param: identifier: If supplied, look for an application instance with the provided identifier - :returns: An instance of ApplicationResponse containing the application data + Returns: + An instance of ApplicationResponse containing the application data. """ identification: Any = id if id is None and identifier is None: @@ -164,8 +170,12 @@ def load_application_data( them from app_data.workflow_file.runtime_config and app_data.template_vars for backward compatibility. - :param: app_data: A dictionary containing the application data - :returns: A tuple containing the application config and the application module + Args: + app_data: A dictionary containing the application data. + application_source_file: The source file of the application. + + Returns: + A tuple containing the application config and the application module. """ if not app_data.workflow_files: # make type checker happy raise Abort( @@ -205,10 +215,13 @@ def load_application_data( @contextlib.contextmanager def get_upload_files(application_path: pathlib.Path): """ - Context manager to build the ``files`` parameter. + Context manager to build the files parameter. - Open the supplied file(s) and build a ``files`` param appropriate for using + Open the supplied file(s) and build a files param appropriate for using multi-part file uploads with the client. + + Args: + application_path: The directory where the application files are located. """ Abort.require_condition(application_path.is_dir(), f"Application directory {application_path} does not exist") @@ -232,11 +245,11 @@ def upload_application( """ Upload an application given an application path and the application id. - :param: jg_ctx: The JobbergateContext. Needed to access the Httpx client with which to - make API calls - :param: application_path: The directory where the application files to upload may be found - :param: application_id: The id of the application for which to upload data - :param: application_identifier: The identifier of the application for which to upload data + Args: + jg_ctx: The JobbergateContext. Needed to access the Httpx client with which to make API calls. + application_path: The directory where the application files to upload may be found. + application_id: The id of the application for which to upload data. + application_identifier: The identifier of the application for which to upload data. """ # Make static type checkers happy @@ -393,8 +406,11 @@ def load_application_config_from_source(config_source: str) -> JobbergateApplica """ Load the JobbergateApplicationConfig from a text string containing the config as YAML. - :param: config_source: The YAML containing the config - :returns: A JobbergateApplicationConfig instance with the config values + Args: + config_source: The YAML containing the config + + Returns: + A JobbergateApplicationConfig instance with the config values """ config_data = yaml.safe_load(config_source) config = JobbergateApplicationConfig(**config_data) @@ -409,8 +425,9 @@ def load_application_from_source(app_source: str, app_config: JobbergateApplicat Adapted from: https://docs.python.org/3/library/importlib.html#importing-a-source-file-directly - :param: app_source: The JobbergateApplication source code to load - :param: app_config: The JobbergateApplicationConfig needed to instantiate the JobbergateApplication + Args: + app_source: The JobbergateApplication source code to load + app_config: The JobbergateApplicationConfig needed to instantiate the JobbergateApplication """ app_locals: Dict[str, Any] = dict() exec(app_source, app_locals, app_locals) @@ -424,10 +441,11 @@ class ApplicationRuntime: """ Prepare and execute a Jobbergate application gathering the answers to the questions. - :param app_data: The application data, can be either an ApplicationResponse or a LocalApplication. - :param app_source_code: The source code of the application, often coming from jobbergate.py file. - :param supplied_params: The parameters supplied to the application, defaults to an empty dictionary. - :param fast_mode: A flag indicating whether the application is in fast mode, defaults to False. + Args: + app_data: The application data, can be either an ApplicationResponse or a LocalApplication. + app_source_code: The source code of the application, often coming from jobbergate.py file. + supplied_params: The parameters supplied to the application, defaults to an empty dictionary. + fast_mode: A flag indicating whether the application is in fast mode, defaults to False. """ app_data: Union[ApplicationResponse, LocalApplication] diff --git a/jobbergate-cli/jobbergate_cli/subapps/job_scripts/tools.py b/jobbergate-cli/jobbergate_cli/subapps/job_scripts/tools.py index 2406023ab..0c870ce70 100644 --- a/jobbergate-cli/jobbergate_cli/subapps/job_scripts/tools.py +++ b/jobbergate-cli/jobbergate_cli/subapps/job_scripts/tools.py @@ -38,9 +38,14 @@ def validate_parameter_file(parameter_path: pathlib.Path) -> Dict[str, Any]: """ Validate parameter file at the supplied path and returns the parsed dict. - Confirms: - parameter_path exists - parameter_path is a valid json file + Args: + parameter_path: Path to the parameter file. + + Raises: + Abort: If file does not exist or is not valid JSON. + + Returns: + Parsed dictionary from the parameter file. """ data = None with Abort.check_expressions( @@ -182,8 +187,12 @@ def render_template( """ Render a template file and save it to the output directory. - :param str template_path: The path to the template file. - :param Dict[str, Any] parameters: The parameters to use for rendering the template. + Args: + template_path: The path to the template file. + parameters: The parameters to use for rendering the template. + + Returns: + The rendered template as a string. """ logger.debug("Rendering template file: {} with parameters={}", template_path, parameters) @@ -234,14 +243,14 @@ def render_job_script_locally( """ Render a new job script from an application in a local directory. - :param str job_script_name: Name of the new job script. - :param pathlib.Path application_path: Path to the base application. - :param pathlib.Path output_path: Path to the output the rendered job script. - :param Optional[List[str]] sbatch_params: List of sbatch parameters. - :param Optional[pathlib.Path] param_file: Path to a parameters file. - :param bool fast: Whether to use default answers (when available) instead of asking the user. - :param JobbergateContext jg_ctx: The Jobbergate context. - :return JobScriptResponse: The new job script. + Args: + jg_ctx: The Jobbergate context. + job_script_name: Name of the new job script. + application_path: Path to the base application. + output_path: Path to the output the rendered job script. + sbatch_params: List of sbatch parameters. + param_file: Path to a parameters file. + fast: Whether to use default answers (when available) instead of asking the user. """ # Make static type checkers happy assert jg_ctx.client is not None @@ -306,15 +315,18 @@ def render_job_script( """ Render a new job script from an application. - :param str name: Name of the new job script. - :param Optional[int] application_id: Id of the base application. - :param Optional[str] application_identifier: Identifier of the base application. - :param Optional[str] description: Description of the new job script. - :param Optional[List[str]] sbatch_params: List of sbatch parameters. - :param Optional[pathlib.Path] param_file: Path to a parameters file. - :param bool fast: Whether to use default answers (when available) instead of asking the user. - :param JobbergateContext jg_ctx: The Jobbergate context. - :return JobScriptResponse: The new job script. + Args: + jg_ctx: The Jobbergate context. + name: Name of the new job script. + application_id: Id of the base application. + application_identifier: Identifier of the base application. + description: Description of the new job script. + sbatch_params: List of sbatch parameters. + param_file: Path to a parameters file. + fast: Whether to use default answers (when available) instead of asking the user. + + Returns: + The new job script. """ # Make static type checkers happy assert jg_ctx.client is not None @@ -395,16 +407,16 @@ def upload_job_script_files( jg_ctx: JobbergateContext, job_script_id: int, job_script_path: pathlib.Path, - supporting_file_paths: Optional[List[pathlib.Path]] = None, + supporting_file_paths: list[pathlib.Path] | None = None, ): """ Upload a job-script and its supporting files given their paths and the job-script id. - :param: jg_ctx: The JobbergateContext. Needed to access the Httpx client with which to make API calls - :param: job_script_path: The path to the job-script file to upload - :param: supporting_file_paths: The paths to any supporting files to upload with the job-scritpt - :param: job_script_id: The id of the job-script for which to upload data - :returns: True if the main job script upload was successful; False otherwise + Args: + jg_ctx: The ``JobbergateContext`` needed to access the Httpx client with which to make API calls + job_script_path: The path to the job-script file to upload + supporting_file_paths: The paths to any supporting files to upload with the job-scritpt + job_script_id: The id of the job-script for which to upload data """ client = JobbergateCliError.enforce_defined(jg_ctx.client) @@ -498,17 +510,20 @@ def question_helper(question_func: Callable, text: str, default: Any, fast: bool """ Helper function for asking questions to the user. - :param Callable question_func: The function to use to ask the question - :param str text: The text of the question to ask - :param Any default: The default value to use if the user does not provide one - :param bool fast: Whether to use default answers (when available) instead of asking the user - :param Any actual_value: The actual value provided by the user, if any + Args: + question_func: The function to use to ask the question + text: The text of the question to ask + default: The default value to use if the user does not provide one + fast: Whether to use default answers (when available) instead of asking the user + actual_value: The actual value provided by the user, if any - :returns: `actual_value` or `default` or the value provided by the user + Returns: + `actual_value` or `default` or the value provided by the user - The `actual_value` has the most priority and will be returned if it is not None. - After evaluating the `actual_value`, the fast mode will determine if the default value will be used. - Otherwise, the question will be prompted to the user. + Notes: + The `actual_value` has the most priority and will be returned if it is not None. + After evaluating the `actual_value`, the fast mode will determine if the default value will be used. + Otherwise, the question will be prompted to the user. """ if actual_value is not None: return actual_value diff --git a/jobbergate-cli/jobbergate_cli/subapps/job_submissions/tools.py b/jobbergate-cli/jobbergate_cli/subapps/job_submissions/tools.py index 3eb4e0f79..236da6e9b 100644 --- a/jobbergate-cli/jobbergate_cli/subapps/job_submissions/tools.py +++ b/jobbergate-cli/jobbergate_cli/subapps/job_submissions/tools.py @@ -22,7 +22,7 @@ def _map_cluster_name( jg_ctx: JobbergateContext, base_cluster_name: str, -): +) -> str: """ Injects the organization name into the cluster name for multi-tenancy mode. @@ -41,19 +41,19 @@ class JobSubmissionABC(ABC): """ A dataclass representing a job submission for Jobbergate. - :param: jg_ctx: The JobbergateContext. Used to retrieve the client for requests - and the email of the submitting user - :param: job_script_id: The ``id`` of the Job Script to submit to Slurm - :param: name: The name to attach to the Job Submission - :param: description: An optional description that may be added to the Job Submission - :param: cluster_name: An optional cluster_name for the cluster where the job should be executed, - If left off, it will default to the DEFAULT_CLUSTER_NAME from the settings. - If no default is set, an exception will be raised. - :param: execution_directory: An optional directory where the job should be executed. If provided as a - relative path, it will be constructed as an absolute path relative to - the current working directory. - :param: download: A flag indicating whether the job script files should be downloaded to the. - :param: sbatch_arguments: An optional list of arguments to pass to inject into the job script. + Args: + jg_ctx: The JobbergateContext. Used to retrieve the client for requests and the email of the submitting user. + job_script_id: The ``id`` of the Job Script to submit to Slurm. + name: The name to attach to the Job Submission. + description: An optional description that may be added to the Job Submission. + cluster_name: An optional cluster_name for the cluster where the job should be executed. + If left off, it will default to the DEFAULT_CLUSTER_NAME from the settings. + If no default is set, an exception will be raised. + execution_directory: An optional directory where the job should be executed. If provided as a + relative path, it will be constructed as an absolute path relative to + the current working directory. + download: A flag indicating whether the job script files should be downloaded to the execution directory. + sbatch_arguments: An optional list of arguments to pass to inject into the job script. """ jg_ctx: JobbergateContext diff --git a/jobbergate-core/jobbergate_core/auth/token.py b/jobbergate-core/jobbergate_core/auth/token.py index 7ecaded57..a2e7bfe63 100644 --- a/jobbergate-core/jobbergate_core/auth/token.py +++ b/jobbergate-core/jobbergate_core/auth/token.py @@ -40,7 +40,7 @@ class TokenData(TypedDict, total=False): @dataclass(frozen=True) class Token: """ - Low-level class used to handling tokens. + Low-level class used to handle tokens. Arguments: cache_directory: The directory used for cache. diff --git a/jobbergate-docs/docs/source/developer_guide/dev_tools.md b/jobbergate-docs/docs/source/developer_guide/dev_tools.md index a54966350..f285457d7 100644 --- a/jobbergate-docs/docs/source/developer_guide/dev_tools.md +++ b/jobbergate-docs/docs/source/developer_guide/dev_tools.md @@ -53,7 +53,7 @@ poetry run dev-tools db --help This command allows you to log in to the database that your Jobbergate API is configured to connect with. It allows you to login to databases, regardless of whether they are -locally hosted via Docker or situated on a remote PostgreSQL server. this ensures +locally hosted via Docker or situated on a remote PostgreSQL server. This ensures seamless access to any database that the Jobbergate API is configured to connect with. To log in to the database, execute this command: diff --git a/jobbergate-docs/docs/source/developer_guide/integration_testing.md b/jobbergate-docs/docs/source/developer_guide/integration_testing.md index c1a4b7dd2..befc498ae 100644 --- a/jobbergate-docs/docs/source/developer_guide/integration_testing.md +++ b/jobbergate-docs/docs/source/developer_guide/integration_testing.md @@ -382,7 +382,7 @@ directory named `/nfs`. The `/nfs` directory in the container is mounted from th `slurm-fake-nfs` directory in the `jobbergate-composed` subproject. You can look in this directory after the job completes execution to check the results. -You will need to verify that jobs are being submitted correctly vai the following steps: +You will need to verify that jobs are being submitted correctly via the following steps: - Submit the job through the CLI - Verify that the agent submitted the job diff --git a/jobbergate-docs/docs/source/developer_guide/template_workflows.md b/jobbergate-docs/docs/source/developer_guide/template_workflows.md index afaaf96b1..d860fcf13 100644 --- a/jobbergate-docs/docs/source/developer_guide/template_workflows.md +++ b/jobbergate-docs/docs/source/developer_guide/template_workflows.md @@ -60,14 +60,9 @@ class. The question types include: - File: prompt the user for a file path - Checkbox: prompt the user to select as many items from a list as they want - Confirm: prompt the user to offer a boolean response -- BooleanList: prompt a series of boolean responses +- BooleanList: Prompt the user for a boolean input, then present a preset list of questions based on their initial response - Const: set the variable to the default value without even asking the user -!!!note - - The BooleanList question has some very complex logic. The source code should be - examined to understand what this does in detail. - All of the implementation of the question classes (including the base class) can be found in [the questions module](https://github.com/omnivector-solutions/jobbergate/blob/main/jobbergate-cli/jobbergate_cli/subapps/applications/questions.py) of the Jobbergate source code.