lablup / lablup/backend.ai

Remove the dead group callback in `client/cli/main.py` and the stale `session/app.py` duplicate

Open
#13,368 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
670
Forks
183
Avg merge
17h 7m
Merged PRs (30d)
358

Description

### Main idea

Follow-up to #13367. While tracing where `--output` is consumed, `src/ai/backend/client/cli/main.py` turned out to declare a group whose options and callback never execute. The correct fix is a **partial** removal — the module and its entry point must stay — plus deletion of one genuinely dead 376-line file.

#### What is actually dead

`src/ai/backend/client/cli/main.py:18-57` — the `--skip-sslcert-validation` / `--output` / `--version` options and the callback that builds a `CLIContext`.

Why they never run:

1. `src/ai/backend/client/BUILD:34` registers the group under the `backendai_cli_v10` entry-point key `_`.
2. `src/ai/backend/cli/loader.py:19-21` handles `_` by copying **only** `cmd_group.commands` into the unified root group. The group's own params and callback are discarded.
3. The only module that ever registered commands on this group is `src/ai/backend/client/cli/session/app.py` (`@main.command()` at lines 268 and 330) — and that module is never imported. `src/ai/backend/client/cli/session/__init__.py` is a 0-byte file, and `client/cli/__init__.py`'s `from . import app, logs, proxy` resolves to the top-level `client/cli/app.py`, not `client/cli/session/app.py`.

So the group is empty, and step 2 copies zero commands. Two independent confirmations:

```console
$ ./py -m ai.backend.client.cli --help # no "Commands:" section at all
$ ./backend.ai apps -h # metavar is SESSION_ID
```

`SESSION_ID` is `client/cli/app.py:331`; `client/cli/session/app.py:330` says `NAME`. The live `apps` command comes from the top-level module.

Every other client command group is registered directly on `ai.backend.cli.main:main` — `client/cli/__init__.py` imports it as `cli_main` and decorates with `@cli_main.group(cls=LazyGroup, ...)`.

#### What must NOT be removed

**The `_` entry point and the `ai.backend.client.cli.main` module.** Loading that entry point is what imports the `ai.backend.client.cli` package, and the package `__init__.py` import side-effect is the *only* thing that registers `service`, `admin`, `vfolder`, `session`, `v2`, … onto the unified root. Drop the entry point and `backend.ai` loses every client command. The loader also type-asserts the loaded object is a `click.Group`, so the name `main` has to keep resolving to one.

So: keep a minimal group as the entry-point anchor, delete the options and the callback body.

#### Proposed changes

| Target | Action |
|---|---|
| `src/ai/backend/client/cli/session/app.py` | **Delete.** Never imported; a stale near-duplicate of `client/cli/app.py` (`diff` shows only cosmetic drift plus `sys.exit(1)` vs `sys.exit(ExitCode.FAILURE)`). |
| `src/ai/backend/client/cli/main.py:18-57` | Reduce to a bare `@click.group(cls=ExtendedCommandGroup)` with a docstring stating it exists solely as the `backendai_cli_v10` `_` anchor. |
| `src/ai/backend/client/cli/extensions.py:46` | `case CLIContext():` in `pass_ctx_obj` becomes provably dead once the callback above is gone — `client/cli/main.py:53` is the only assignment of a client `CLIContext` to `ctx.obj` anywhere in the tree. Removable, though see below. |

#### Alternative ideas

- **Restore it instead of removing it** — i.e. make `python -m ai.backend.client.cli` a working client-only CLI again by registering the client groups on it and having the unified CLI mount that group. Rejected: it means every client group gains a second registration root, and `--output` would then need resolving in two places, which is the exact confusion #13367 is trying to remove. There is no shipped artifact that needs a client-only entry point — `pex_binary(name="backendai-client")` uses `common_scie_config`'s default `entry_point="ai.backend.cli.__main__"`, so even the standalone client scie goes through the unified root.
- **Keep `case CLIContext():`** as a deliberate extension point for third-party CLI plugins that build their own `CLIContext`. `src/ai/backend/plugin/entrypoint.py:88` does allow out-of-tree `backendai_cli_v10` providers, so this is a judgement call rather than a clear-cut deletion; if kept, it should carry a comment saying why.
- **Delete `client/cli/__main__.py` too.** It currently produces a CLI with zero commands. Left out of the proposal above because it is two lines and harmless; fold it in if the anchor group gets a clearer name.

### Anything else?

Regression coverage already has a home: `tests/unit/client/cli/conftest.py` exposes a `cli_entrypoint` fixture built from `load_entry_points(allowlist={"ai.backend.client.cli"})`. A test asserting that the loaded root still exposes `service` / `admin` / `vfolder` / `app` / `apps` would pin down the import side-effect that this cleanup must preserve — that is the one way this change can break silently.

Contributor guide

Open the contributing guide

Research direction

Start by reading src/ai/backend/client/cli/main.py, src/ai/backend/cli/loader.py, and src/ai/backend/client/cli/__init__.py to verify how the `_` entry point preserves command registration. Remove the unused session/app.py duplicate and reduce the anchor group without removing its entry point; review the CLIContext case separately because the issue leaves that decision open. Run the cli_entrypoint coverage from tests/unit/client/cli/conftest.py and confirm service, admin, vfolder, app, and apps remain available.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
cli, tooling
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
70/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.