Skip to content

Commit

Permalink
solve feedback on acceptance (#613)
Browse files Browse the repository at this point in the history
migrated docstrings from sphinx to google style

renamed function garbage_collect to garbage_collector

Fixed spelling errors in the docstrings
  • Loading branch information
fschuch authored Sep 27, 2024
1 parent 8418de0 commit d89a13e
Show file tree
Hide file tree
Showing 23 changed files with 287 additions and 227 deletions.
9 changes: 5 additions & 4 deletions jobbergate-agent/jobbergate_agent/jobbergate/submit.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}")

Expand Down Expand Up @@ -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...")

Expand Down
2 changes: 1 addition & 1 deletion jobbergate-agent/jobbergate_agent/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
60 changes: 29 additions & 31 deletions jobbergate-agent/jobbergate_agent/utils/exception.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions jobbergate-agent/jobbergate_agent/utils/logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
23 changes: 15 additions & 8 deletions jobbergate-api/jobbergate_api/apps/file_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion jobbergate-api/jobbergate_api/apps/garbage_collector.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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],
Expand Down
4 changes: 2 additions & 2 deletions jobbergate-api/jobbergate_api/apps/job_scripts/routers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
4 changes: 2 additions & 2 deletions jobbergate-api/jobbergate_api/apps/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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.
"""
Expand Down
4 changes: 2 additions & 2 deletions jobbergate-api/tests/apps/test_garbage_collector.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion jobbergate-cli/jobbergate_cli/logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 2 additions & 1 deletion jobbergate-cli/jobbergate_cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"""

import sys
from typing import Any

import httpx
import importlib_metadata
Expand Down Expand Up @@ -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"

Expand Down
68 changes: 37 additions & 31 deletions jobbergate-cli/jobbergate_cli/render.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand All @@ -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"])
Expand Down Expand Up @@ -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 = []
Expand All @@ -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)
Expand Down
Loading

0 comments on commit d89a13e

Please sign in to comment.