Skip to content

Commit

Permalink
feat: add base_command decorator (#22)
Browse files Browse the repository at this point in the history
- chore: refactor project structure
- feat: add base_command decorator
- test: restructure and complete the test suite
- fix: test coverage report generation
- fix: publication to PyPiTest

# Motivation

The aim of these changes is to provide a simple and elegant way of
integrating the essential basic logic of error handling and common
options into a CLI command. These changes also complement unit testing.

# Description
 
All the CLI commands implemented so far share a number of common
features. Firstly, they all use the same error handling logic (using the
@error_handler decorator). Secondly, all these commands share a number
of common options, such as the cluster endpoint (--endpoint), the output
format option (--output) and the debug flag (--debug). For the moment,
implementing these behaviors requires the addition of numerous
decorators above each function defining a command.

The aim of these changes is to streamline the code by providing a single
decorator that abstracts this complexity.
The aim is to centralize this logic to lighten the code and simplify
future modifications.

In addition, a number of unit tests are missing. These changes add a
number of them and significantly improve test coverage.

# Testing

A number of unit tests have been added to ensure that the changes made
do not alter the proper operation of the CLI. In addition, manual tests
have been performed to validate end-to-end functionality.

# Impact

These changes have no impact on the rest of the ArmoniK project. They
are purely internal to this project.

# Checklist

- [x] My code adheres to the coding and style guidelines of the project.
- [x] I have performed a self-review of my code.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [ ] I have made corresponding changes to the documentation.
- [x] I have thoroughly tested my modifications and added tests when
necessary.
- [x] Tests pass locally and in the CI.
- [x] I have assessed the performance impact of my modifications.
  • Loading branch information
ngruelaneo authored Dec 2, 2024
2 parents 9621d33 + 943ac77 commit 3aa2315
Show file tree
Hide file tree
Showing 19 changed files with 523 additions and 191 deletions.
3 changes: 3 additions & 0 deletions .coveragerc
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[run]
omit =
*/_version.py
8 changes: 4 additions & 4 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -70,22 +70,22 @@ jobs:
python -m uv pip install .[tests]
- name: Testing
run: pytest tests --cov=armonik_cli --cov-report=term-missing
run: pytest tests --cov=armonik_cli --cov-config=.coveragerc --cov-report=term-missing --cov-append --cov-report xml:coverage.xml --cov-report html:coverage_report

- name: Get Report
uses: orgoro/coverage@3f13a558c5af7376496aa4848bf0224aead366ac
with:
coverageFile: packages/python/coverage.xml
coverageFile: coverage.xml
token: ${{ secrets.GITHUB_TOKEN }}

- name: Archive code coverage results html
uses: actions/upload-artifact@834a144ee995460fba8ed112a2fc961b36a5ec5a
with:
name: code-coverage-report-html
path: packages/python/coverage_report
path: coverage_report

- name: Archive code coverage results xml
uses: actions/upload-artifact@834a144ee995460fba8ed112a2fc961b36a5ec5a
with:
name: code-coverage-report-xml
path: packages/python/coverage.xml
path: coverage.xml
3 changes: 1 addition & 2 deletions .github/workflows/publish.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
name: Python Package

on:
push:
pull_request:
branches:
- main
Expand Down Expand Up @@ -46,7 +45,7 @@ jobs:
packages-dir: dist/
repository-url: https://test.pypi.org/legacy/

- name: Publish to PyPiTest
- name: Publish to PyPi
if: github.event_name == 'release' # Publish on releases
uses: pypa/gh-action-pypi-publish@release/v1
with:
Expand Down
71 changes: 18 additions & 53 deletions src/armonik_cli/commands/sessions.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,32 +7,21 @@
from armonik.client.sessions import ArmoniKSessions
from armonik.common import SessionStatus, Session, TaskOptions

from armonik_cli.console import console
from armonik_cli.errors import error_handler
from armonik_cli.commands.common import (
endpoint_option,
output_option,
debug_option,
KeyValuePairParam,
TimeDeltaParam,
)
from armonik_cli.core import console, base_command, KeyValuePairParam, TimeDeltaParam


SESSION_TABLE_COLS = [("ID", "SessionId"), ("Status", "Status"), ("CreatedAt", "CreatedAt")]
session_argument = click.argument("session-id", required=True, type=str, metavar="SESSION_ID")


@click.group(name="sessions")
@click.group(name="session")
def sessions() -> None:
"""Manage cluster sessions."""
pass


@sessions.command()
@endpoint_option
@output_option
@debug_option
@error_handler
@base_command
def list(endpoint: str, output: str, debug: bool) -> None:
"""List the sessions of an ArmoniK cluster."""
with grpc.insecure_channel(endpoint) as channel:
Expand All @@ -43,15 +32,13 @@ def list(endpoint: str, output: str, debug: bool) -> None:
sessions = [_clean_up_status(s) for s in sessions]
console.formatted_print(sessions, format=output, table_cols=SESSION_TABLE_COLS)

console.print(f"\n{total} sessions found.")
# TODO: Use logger to display this information
# console.print(f"\n{total} sessions found.")


@sessions.command()
@endpoint_option
@output_option
@debug_option
@session_argument
@error_handler
@base_command
def get(endpoint: str, output: str, session_id: str, debug: bool) -> None:
"""Get details of a given session."""
with grpc.insecure_channel(endpoint) as channel:
Expand All @@ -62,7 +49,6 @@ def get(endpoint: str, output: str, session_id: str, debug: bool) -> None:


@sessions.command()
@endpoint_option
@click.option(
"--max-retries",
type=int,
Expand Down Expand Up @@ -130,9 +116,7 @@ def get(endpoint: str, output: str, session_id: str, debug: bool) -> None:
help="Additional default options.",
metavar="KEY=VALUE",
)
@output_option
@debug_option
@error_handler
@base_command
def create(
endpoint: str,
max_retries: int,
Expand Down Expand Up @@ -173,12 +157,9 @@ def create(


@sessions.command()
@endpoint_option
@click.confirmation_option("--confirm", prompt="Are you sure you want to cancel this session?")
@output_option
@debug_option
@session_argument
@error_handler
@base_command
def cancel(endpoint: str, output: str, session_id: str, debug: bool) -> None:
"""Cancel a session."""
with grpc.insecure_channel(endpoint) as channel:
Expand All @@ -189,11 +170,8 @@ def cancel(endpoint: str, output: str, session_id: str, debug: bool) -> None:


@sessions.command()
@endpoint_option
@output_option
@debug_option
@session_argument
@error_handler
@base_command
def pause(endpoint: str, output: str, session_id: str, debug: bool) -> None:
"""Pause a session."""
with grpc.insecure_channel(endpoint) as channel:
Expand All @@ -204,11 +182,8 @@ def pause(endpoint: str, output: str, session_id: str, debug: bool) -> None:


@sessions.command()
@endpoint_option
@output_option
@debug_option
@session_argument
@error_handler
@base_command
def resume(endpoint: str, output: str, session_id: str, debug: bool) -> None:
"""Resume a session."""
with grpc.insecure_channel(endpoint) as channel:
Expand All @@ -219,12 +194,9 @@ def resume(endpoint: str, output: str, session_id: str, debug: bool) -> None:


@sessions.command()
@endpoint_option
@click.confirmation_option("--confirm", prompt="Are you sure you want to close this session?")
@output_option
@debug_option
@session_argument
@error_handler
@base_command
def close(endpoint: str, output: str, session_id: str, debug: bool) -> None:
"""Close a session."""
with grpc.insecure_channel(endpoint) as channel:
Expand All @@ -235,12 +207,9 @@ def close(endpoint: str, output: str, session_id: str, debug: bool) -> None:


@sessions.command()
@endpoint_option
@click.confirmation_option("--confirm", prompt="Are you sure you want to purge this session?")
@output_option
@debug_option
@session_argument
@error_handler
@base_command
def purge(endpoint: str, output: str, session_id: str, debug: bool) -> None:
"""Purge a session."""
with grpc.insecure_channel(endpoint) as channel:
Expand All @@ -251,12 +220,9 @@ def purge(endpoint: str, output: str, session_id: str, debug: bool) -> None:


@sessions.command()
@endpoint_option
@click.confirmation_option("--confirm", prompt="Are you sure you want to delete this session?")
@output_option
@debug_option
@session_argument
@error_handler
@base_command
def delete(endpoint: str, output: str, session_id: str, debug: bool) -> None:
"""Delete a session and associated data from the cluster."""
with grpc.insecure_channel(endpoint) as channel:
Expand All @@ -267,8 +233,6 @@ def delete(endpoint: str, output: str, session_id: str, debug: bool) -> None:


@sessions.command()
@endpoint_option
@session_argument
@click.option(
"--clients-only",
is_flag=True,
Expand All @@ -281,9 +245,8 @@ def delete(endpoint: str, output: str, session_id: str, debug: bool) -> None:
default=False,
help="Prevent only workers from submitting new tasks in the session.",
)
@output_option
@debug_option
@error_handler
@session_argument
@base_command
def stop_submission(
endpoint: str, session_id: str, clients_only: bool, workers_only: bool, output: str, debug: bool
) -> None:
Expand All @@ -293,7 +256,9 @@ def stop_submission(
session = sessions_client.stop_submission_session(
session_id=session_id, client=clients_only, worker=workers_only
)
console.formatted_print(session, format=output, table_cols=SESSION_TABLE_COLS)
console.formatted_print(
_clean_up_status(session), format=output, table_cols=SESSION_TABLE_COLS
)


def _clean_up_status(session: Session) -> Session:
Expand Down
6 changes: 6 additions & 0 deletions src/armonik_cli/core/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
from armonik_cli.core.console import console
from armonik_cli.core.decorators import base_command
from armonik_cli.core.params import KeyValuePairParam, TimeDeltaParam


__all__ = ["base_command", "KeyValuePairParam", "TimeDeltaParam", "console"]
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from rich.console import Console
from rich.table import Table

from armonik_cli.utils import CLIJSONEncoder
from armonik_cli.core.serialize import CLIJSONEncoder


class ArmoniKCLIConsole(Console):
Expand Down
102 changes: 102 additions & 0 deletions src/armonik_cli/core/decorators.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
from functools import wraps, partial

import grpc
import rich_click as click

from armonik_cli.core.console import console
from armonik_cli.exceptions import NotFoundError, InternalError


def error_handler(func=None):
"""Decorator to ensure correct display of errors.
Args:
func: The command function to be decorated. If None, a partial function is returned,
allowing the decorator to be used with parentheses.
Returns:
The wrapped function with added CLI options.
"""
# Allow to call the decorator with parenthesis.
if not func:
return partial(error_handler)

@wraps(func)
def wrapper(*args, **kwargs):
try:
return func(*args, **kwargs)
except click.ClickException:
raise
except grpc.RpcError as err:
status_code = err.code()
error_details = f"{err.details()}."

if status_code == grpc.StatusCode.NOT_FOUND:
raise NotFoundError(error_details)
else:
raise InternalError("An internal fatal error occured.")
except Exception:
if "debug" in kwargs and kwargs["debug"]:
console.print_exception()
else:
raise InternalError("An internal fatal error occured.")

return wrapper


def base_command(func=None):
"""Decorator to add common CLI options to a Click command function, including
'endpoint', 'output', and 'debug'. These options are automatically passed
as arguments to the decorated function.
The following options are added to the command:
- `--endpoint` (required): Specifies the cluster endpoint.
- `--output`: Sets the output format, with options 'yaml', 'json', or 'table' (default is 'json').
- `--debug`: Enables debug mode, printing additional logs if set.
Warning:
If the decorated function has parameters with the same names as the options added by
this decorator, this can lead to conflicts and unpredictable behavior.
Args:
func: The command function to be decorated. If None, a partial function is returned,
allowing the decorator to be used with parentheses.
Returns:
The wrapped function with added CLI options.
"""

# Allow to call the decorator with parenthesis.
if not func:
return partial(base_command)

# Define the wrapper function with added Click options
@click.option(
"-e",
"--endpoint",
type=str,
required=True,
help="Endpoint of the cluster to connect to.",
metavar="ENDPOINT",
)
@click.option(
"-o",
"--output",
type=click.Choice(["yaml", "json", "table"], case_sensitive=False),
default="json",
show_default=True,
help="Commands output format.",
metavar="FORMAT",
)
@click.option(
"--debug", is_flag=True, default=False, help="Print debug logs and internal errors."
)
@error_handler
@wraps(func)
def wrapper(endpoint: str, output: str, debug: bool, *args, **kwargs):
kwargs["endpoint"] = endpoint
kwargs["output"] = output
kwargs["debug"] = debug
return func(*args, **kwargs)

return wrapper
Original file line number Diff line number Diff line change
Expand Up @@ -6,28 +6,6 @@
from typing import cast, Tuple, Union


endpoint_option = click.option(
"-e",
"--endpoint",
type=str,
required=True,
help="Endpoint of the cluster to connect to.",
metavar="ENDPOINT",
)
output_option = click.option(
"-o",
"--output",
type=click.Choice(["yaml", "json", "table"], case_sensitive=False),
default="json",
show_default=True,
help="Commands output format.",
metavar="FORMAT",
)
debug_option = click.option(
"--debug", is_flag=True, default=False, help="Print debug logs and internal errors."
)


class KeyValuePairParam(click.ParamType):
"""
A custom Click parameter type that parses a key-value pair in the format "key=value".
Expand Down
File renamed without changes.
Loading

0 comments on commit 3aa2315

Please sign in to comment.