Skip to content

ref(cli): make attach-partitions output legible for long runs - #8432

Merged
onewland merged 2 commits into
masterfrom
oliver/partition-management-improvements
Sep 3, 2026
Merged

ref(cli): make attach-partitions output legible for long runs#8432
onewland merged 2 commits into
masterfrom
oliver/partition-management-improvements

Conversation

@onewland

@onewland onewland commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

attach-partitions walks partitions one at a time and can run for a long
time, but its progress output was hard to act on. Two commits fix that.

State the mode and tables

The output never said whether a run was a dry run, so the only signal was
the wording of each line, and Would attach partition 90-20240212 named
neither the source nor the destination. Reading a scrollback, you could not
tell a rehearsal from a real attach, or which tables were involved.

The mode and table pair are now announced before any work, and every
partition line is qualified with both tables.

Add timestamps

Progress went through click.echo, so no line carried a timestamp. On a long
run there was no way to tell how long an individual ATTACH took, or when a
run stalled.

The output now goes through a module logger, matching the convention in the
rest of snuba.cli. LOG_FORMAT is "%(asctime)s %(message)s", so lines are
timestamped:

2026-09-03 12:48:40,815 [EXECUTE] Attaching partitions from d.src to d.dst
2026-09-03 12:48:40,815 Attached partition 90-20260608 from d.src to d.dst
2026-09-03 12:48:40,815 [EXECUTE] Attached 1 partition(s) from d.src to d.dst

Behaviour changes to be aware of

Moving to logging has two consequences beyond formatting:

  • Output moves from stdout to stderr, since logging.basicConfig installs
    a StreamHandler on stderr. This would break any pipeline parsing this
    command's stdout. I am not aware of one, but say the word and I will attach a
    stdout handler instead.
  • --log-level now gates these lines, so --log-level WARNING silences
    progress output.

Also worth a look: a --partition-id run now logs twice per partition, because
attach_partition_from_table already emits its own "Attaching partition" line.
Previously the two went to different streams so the overlap was invisible. Happy
to drop the CLI line or demote the library one to debug.

Testing

tests/clickhouse/test_partition_management.py passes (43 tests); mypy and the
pre-commit ruff hooks are clean. No test asserted on this CLI's output before
or after.

The output did not say whether a run was a dry run, so the only signal
was the wording of each line, and "Would attach partition 90-20240212"
named neither the source nor the destination. An operator reading a
scrollback could not tell a rehearsal from a real attach, or which
tables were involved.

Announce the mode and the table pair before any work, and qualify every
partition line with the source and destination.
attach-partitions reported progress with click.echo, so lines like
"Attached partition 90-20260608 ..." and "Attached 1 partition(s) ..."
carried no timestamps. On long runs that made it impossible to tell how
long an individual ATTACH took, or when a run stalled.

Route the output through a module logger instead, matching the
convention in the rest of snuba.cli. LOG_FORMAT is
"%(asctime)s %(message)s", so every line is now timestamped:

  2026-09-03 12:48:40,815 Attached partition 90-20260608 from d.src to d.dst
  2026-09-03 12:48:40,815 [EXECUTE] Attached 1 partition(s) from d.src to d.dst

Two behaviour changes follow from using logging:

- The output moves from stdout to stderr, since logging.basicConfig
  installs a StreamHandler on stderr.
- --log-level now gates these lines, so --log-level WARNING silences
  them.
@onewland
onewland requested a review from a team as a code owner September 3, 2026 19:50
@onewland
onewland enabled auto-merge (squash) September 3, 2026 19:54
@onewland
onewland merged commit 3170625 into master Sep 3, 2026
65 of 66 checks passed
@onewland
onewland deleted the oliver/partition-management-improvements branch September 3, 2026 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants