lablup / lablup/backend.ai

v2 CLI and SDK cannot clear a nullable field - the explicit null is stripped before the request is sent

Open
#13,299 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

## Problem

No v2 CLI or SDK caller can clear a nullable field. The flags built for exactly that are silent no-ops - they return success and change nothing.

```
./bai admin idle-checker update --clear-description
-> description unchanged, no error

PATCH /v2/idle-checkers/ {"id": "...", "description": null}
-> description cleared
```

The server is correct. 60 of the 61 sentinel branches in api/adapters/ already distinguish the three input states - omitted keeps the stored value, an explicit null clears it, a value sets it. The null never reaches them.

## Root cause

client/v2/base_client.py serializes every request with exclude_none=True at three call sites (lines 179, 206, 459). The explicit null is stripped before the body is sent, so the server sees an absent key and falls back to the DTO default of SENTINEL, i.e. "no change".

exclude_none is load-bearing, not an oversight. The v2 CLI passes every field explicitly to the DTO constructor, so exclude_unset filters nothing, and exclude_none is what translates click's "option not given -> None" into "key absent on the wire". Removing it alone turns every omitted CLI option into a destructive null.

## Affected

79 Sentinel-typed fields across 24 v2 DTO modules. Four clear-this-field affordances are exposed today and all four are broken:

- ./bai admin idle-checker update --clear-description
- ./bai deployment revision add --no-shell (client/cli/v2/deployment/revision.py)
- ./bai artifact update --description (clearing)
- ./bai object-storage update --region (clearing)

admin/runtime_variant.py update already documents the intended contract in its docstring - "pass an explicit null to clear a nullable field" - which the transport currently breaks.

## Constraint on the fix

Dropping exclude_none requires the CLI to mark "not provided" with SENTINEL rather than None, and SENTINEL must then be removed from the body. It cannot simply be serialized: Sentinel.TOKEN is enum.auto(), so mode="json" renders it as the integer 1, and an int-typed field re-parses that as a value rather than as the sentinel.

```
class Plain(BaseModel):
max_pending_session_count: int | Sentinel | None = SENTINEL

Plain(max_pending_session_count=SENTINEL).model_dump(mode="json", exclude_unset=True)
-> {"max_pending_session_count": 1}
server re-validates -> 1 # the integer, not SENTINEL
```

So omitting an int option would silently set it to 1. Sentinel-typed int fields include max_pending_session_count, max_vfolder_count, max_quota_scope_size, max_concurrent_logins, min_replicas, max_replicas, container_uid, replica_count and rank. String fields survive by accident because 1 does not match str.

## Proposed direction

- Strip SENTINEL-valued keys in the transport layer (client/v2/base_client.py), recursively - BulkUpdateUserItemInput nests UpdateUserInput, which carries 9 Sentinel fields, so a top-level exclude set is not enough
- Have the CLI pass SENTINEL, not None, for a Sentinel-typed field whose option was not given - only 3 of the 18 CLI files need this; the rest either build the DTO from a JSON body or already filter None
- Then remove exclude_none=True from the three base_client call sites
- Keep exclude_unset=True so a direct SDK caller that never sets a field is unaffected

## Acceptance Criteria

- An explicit null sent through the v2 SDK reaches the server and clears the column
- An omitted CLI option leaves the stored value unchanged, including for int-typed Sentinel fields
- SENTINEL never appears in a request body in any form
- --clear-description and --no-shell take effect, verified against a live manager
- A regression test walks every v2 request DTO and fails, naming each offender, if a Sentinel-typed field does not default to SENTINEL or if the three states are not distinct after serialization
- CI quality gates (format, lint, type check, unit suite) are green

## References

- src/ai/backend/client/v2/base_client.py:179,206,459
- src/ai/backend/client/cli/v2/admin/login_client_type.py, admin/prometheus_query_preset.py, deployment/auto_scaling_rule.py - the three that pass None
- src/ai/backend/common/api_handlers.py - Sentinel / SENTINEL
- BA-7101 - the same three-state defect pointing the other way, on the GraphQL input side
- CHECKLIST_26.8 item 12 - found while checking deployment revision add --no-shell

JIRA Issue: BA-7104

Contributor guide

Open the contributing guide

Research direction

Start with the three serialization call sites in src/ai/backend/client/v2/base_client.py at lines 179, 206, and 459, then inspect Sentinel/SENTINEL in src/ai/backend/common/api_handlers.py. Review the three CLI files named in the issue and trace the affected clear options. Done means explicit null is sent, omitted options remain unchanged, SENTINEL is absent recursively, and the listed regression and quality checks pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
api, backend-api-design, cli, testing
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.