Refactor all of CLI

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.

cvat-cli/src/cvat_cli/__main__.py

@@ -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
 

cvat-cli/src/cvat_cli/_internal/command_base.py

@@ -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)
+            command.configure_parser(subparser)
+            subparser.set_defaults(executor=command.execute)
+
+    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"