Skip to content

Commit

Permalink
Refactor all of CLI
Browse files Browse the repository at this point in the history
The main goal for this refactoring is to co-locate all code responsible for
each command. Currently, such code is spread across 3 places:

* parser configuration in `parser.py`;
* implementation in `cli.py`;
* name-to-code mapping in `__main__.py`.

This patch combines all three aspects of each command in a single class
following a common protocol. This makes it easier to add/modify commands.

In addition, I made the following changes:

* Moved most code into an `_internal` package, to discourage external code
  from using it.

* Split off some code into dedicated modules.

* Added a `CommandGroup` class that itself implements the `Command`
  protocol. This makes it possible to implement command hierarchies in the
  future.

* Redid the somewhat hacky way in which the code determined which options
  to pass to the command implementation. Now we pop all common options when
  we process them, which means all remaining options belong to the command.
  I had to mark some of the `create` options with
  `default=argparse.SUPPRESS` to make this work.

* Replaced the descriptions for some of the commands with the docstring of
  the implementation functions, which had more details. Removed the
  docstrings themselves, which were redundant.

* Removed the `RawTextHelpFormatter` from the `create` argument parser, to
  ensure consistent help formatting between commands. Adjusted the
  `--filename_pattern` option description to compensate.

* Cleaned up the signatures of implementation functions: made all parameters
  except the client kw-only, removed default values (which would never be
  used), removed parameters that didn't correspond to any command-line
  options, fixed some parameter types.

Other than the command description updates, none of the changes here should
affect the external behavior of the CLI.
  • Loading branch information
SpecLad committed Nov 4, 2024
1 parent 9ecafb6 commit f5664ee
Show file tree
Hide file tree
Showing 9 changed files with 743 additions and 687 deletions.
75 changes: 17 additions & 58 deletions cvat-cli/src/cvat_cli/__main__.py
Original file line number Diff line number Diff line change
@@ -1,79 +1,38 @@
# Copyright (C) 2020-2022 Intel Corporation
# Copyright (C) 2022 CVAT.ai Corporation
# Copyright (C) 2022-2024 CVAT.ai Corporation
#
# SPDX-License-Identifier: MIT

import argparse
import logging
import sys
from http.client import HTTPConnection
from types import SimpleNamespace
from typing import List

import urllib3.exceptions
from cvat_sdk import exceptions
from cvat_sdk.core.client import Client, Config

from cvat_cli.cli import CLI
from cvat_cli.parser import get_action_args, make_cmdline_parser
from ._internal.commands import COMMANDS
from ._internal.common import build_client, configure_common_arguments, configure_logger
from ._internal.utils import popattr

logger = logging.getLogger(__name__)


def configure_logger(level):
formatter = logging.Formatter(
"[%(asctime)s] %(levelname)s: %(message)s", datefmt="%Y-%m-%d %H:%M:%S", style="%"
)
handler = logging.StreamHandler(sys.stdout)
handler.setFormatter(formatter)
logger.addHandler(handler)
logger.setLevel(level)
if level <= logging.DEBUG:
HTTPConnection.debuglevel = 1


def build_client(parsed_args: SimpleNamespace, logger: logging.Logger) -> Client:
config = Config(verify_ssl=not parsed_args.insecure)

url = parsed_args.server_host
if parsed_args.server_port:
url += f":{parsed_args.server_port}"

client = Client(
url=url,
logger=logger,
config=config,
check_server_version=False, # version is checked after auth to support versions < 2.3
)

client.organization_slug = parsed_args.organization

return client


def main(args: List[str] = None):
actions = {
"create": CLI.tasks_create,
"delete": CLI.tasks_delete,
"ls": CLI.tasks_list,
"frames": CLI.tasks_frames,
"dump": CLI.tasks_dump,
"upload": CLI.tasks_upload,
"export": CLI.tasks_export,
"import": CLI.tasks_import,
"auto-annotate": CLI.tasks_auto_annotate,
}
parser = make_cmdline_parser()
parser = argparse.ArgumentParser(description=COMMANDS.description)
configure_common_arguments(parser)
COMMANDS.configure_parser(parser)

parsed_args = parser.parse_args(args)
configure_logger(parsed_args.loglevel)

with build_client(parsed_args, logger=logger) as client:
action_args = get_action_args(parser, parsed_args)
try:
cli = CLI(client=client, credentials=parsed_args.auth)
actions[parsed_args.action](cli, **vars(action_args))
except (exceptions.ApiException, urllib3.exceptions.HTTPError) as e:
logger.critical(e)
return 1
configure_logger(logger, parsed_args)

try:
with build_client(parsed_args, logger=logger) as client:
popattr(parsed_args, "_executor")(client, **vars(parsed_args))
except (exceptions.ApiException, urllib3.exceptions.HTTPError) as e:
logger.critical(e)
return 1

return 0

Expand Down
Empty file.
52 changes: 52 additions & 0 deletions cvat-cli/src/cvat_cli/_internal/command_base.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# Copyright (C) 2024 CVAT.ai Corporation
#
# SPDX-License-Identifier: MIT

import argparse
import types
from typing import Callable, Dict, Mapping, Protocol


class Command(Protocol):
@property
def description(self) -> str: ...

def configure_parser(self, parser: argparse.ArgumentParser) -> None: ...

# The exact parameters accepted by `execute` vary between commands,
# so we're forced to declare it like this instead of as a method.
@property
def execute(self) -> Callable[..., None]: ...


class CommandGroup:
def __init__(self, *, description: str) -> None:
self._commands: Dict[str, Command] = {}
self.description = description

def command_class(self, name: str):
def decorator(cls: type):
self._commands[name] = cls()
return cls

return decorator

def add_command(self, name: str, command: Command) -> None:
self._commands[name] = command

@property
def commands(self) -> Mapping[str, Command]:
return types.MappingProxyType(self._commands)

def configure_parser(self, parser: argparse.ArgumentParser) -> None:
subparsers = parser.add_subparsers(required=True)

for name, command in self._commands.items():
subparser = subparsers.add_parser(name, description=command.description)
subparser.set_defaults(_executor=command.execute)
command.configure_parser(subparser)

def execute(self) -> None:
# It should be impossible for a command group to be executed,
# because configure_parser requires that a subcommand is specified.
assert False, "unreachable code"
Loading

0 comments on commit f5664ee

Please sign in to comment.