ClickHouse / ClickHouse/clickhousectl
Reject empty service backup-configuration updates
- Dominant language
- Rust
- Stars
- 74
- Forks
- 5
- Avg merge
- 2d 11h
- Merged PRs (30d)
- 196
Description
## Validated scope and current-PR plan — 2026-09-11
This section records the QA review and the current decision to finalize existing PRs without opening new PRs. It supersedes the proposed routing/fix suggestions in the original report below; the original observations are retained. Central plan: #757.
### Work for the existing PRs
Implementation status: the checked sub-items below are implemented on the existing PR branches and validated locally on combined head `82b374816a917cbac3a68194659beaaa5c7c9087`. Required final-head Cloud CI and merging remain pending; this issue stays open. See #757 for the combined validation and CI status.
- [x] #781 — Include the settings-file path in file-open/read diagnostics; cover missing input before organization lookup or HTTP.
- [x] #811 — Make an API-supplied empty setting string visible in human output. Preserve the JSON value; do not reinterpret it as unset/null.
### Remaining scope
Lifecycle wording, semantic unset representation, DELETE/list JSON shapes (#863), backup no-op behavior, tag precedence, upgrade-window/backup presentation and table columns remain deferred.
This is a partial assignment. Keep the issue open after these PRs land; use sub-item references rather than a whole-issue closing keyword.
### Validation and corrections
Reproduced the pathless settings-file error. Mock GET shows the CLI preserves the API’s value:""; #787 did not invent unset encoding and does not own DELETE output. Do not coerce every empty string to null without a documented unset sentinel. Raw DELETE envelope, list wrapper and other JSON changes need an explicit contract decision (#863). No-flag backup update and lifecycle wording are separate handler improvements; tag precedence is a separate behavioral decision, not cosmetic output.
---
## Original QA report
Found in the 2026-09-11 QA sweep of the combined PR stack at [`d2c37f81`](https://github.com/ClickHouse/clickhousectl/commit/d2c37f814fff53fe68d636e34103e2d0550bf84d), top PR #823, installed release build (package version still 0.4.2). Reproduced behaviour only; no implementation included.
Severity: low-medium; batch.
```sh
clickhousectl cloud service start "$SVC" # already running
# Service qa starting (state: running) exit 0 ← misleading if scripted on the message
clickhousectl cloud service stop "$SVC" # already stopped
# Service qa stopping (state: stopped) exit 0
clickhousectl cloud service settings get "$SVC" max_query_size # not configured
# name: max_query_size (value silently dropped)
clickhousectl cloud service settings get "$SVC" max_query_size --json
# {"name":"max_query_size","value":""} (empty string for an integer setting ≠ unset)
clickhousectl cloud service settings unset "$SVC" input_format_try_infer_datetimes --json
# {"status":200,"requestId":"…"} (human: "Reset ClickHouse setting … to its platform default")
clickhousectl cloud service settings set "$SVC" --settings-file /tmp/missing.json
# Error: No such file or directory (os error 2) (every sibling path names the file)
clickhousectl cloud service backup-config update "$SVC" # no flags
# Backup configuration updated (nothing changed)
```
Also: `--add-tag x=1 --remove-tag x=1` in one call warns the remove matched nothing (evaluated on pre-state) then leaves `x=1`; `upgrade-window get` prints `weekday: 3` / `duration: 6` with no day name or unit; `backup list` human `Created` uses `+00:00` while every other timestamp uses `Z`; `settings list --json` wraps in `{"settings":[…]}` while `service list`/`backup list` are bare arrays; the human `service list` table has no Tags column although `--filter tag:` is the only filter.
Suggested: idempotent wording ("already running"); `value` omitted/`null` when unset; `{"name":…,"reset":true}` for unset; include the path in the open error; reject no-flag `backup-config update` (or say "no changes").
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with the QA finalization section and central plan #757, then review existing PRs #781 and #811 and combined head 82b374816a917cbac3a68194659beaaa5c7c9087. Those sub-items are implemented and validated, while the remaining backup, lifecycle, JSON, tag, and presentation topics are explicitly deferred; this issue is not a standalone task with a single defined completion point.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- cli
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 20/100