Skip to content

Report what the install is doing, and unwtar in parallel - #89

Open
MatanTiram wants to merge 1 commit into
master3from
review-prep/install-lifecycle
Open

MatanTiram wants to merge 1 commit into
master3from
review-prep/install-lifecycle

Conversation

@MatanTiram

Copy link
Copy Markdown
Collaborator

An install announces almost nothing between "started" and "done". Central drives its progress bar by parsing free text, and whole phases — reading the index, scanning the sync cache, verifying, copying — pass with no signal a UI can act on. A run that fails emits nothing terminal at all, so a broken install and a hung one look identical from outside.

This adds the reporting, and one real speedup on the way past.

26 files, +3,622 / −66 — of which +1,925 is production code and +1,697 is tests.

The state machine gets an entry and an exit

  • preparing at the top of the client flow for sync/copy/synccopy — covering the yaml and info-map read, item calculation, and the sync-folder scan. That stretch previously reported nothing at all.
  • failed from PythonBatchRuntime.log_error, so a failed install is no longer indistinguishable from a hang. This is the one I would most like a second opinion on: it is a new terminal state on the wire.
  • ready_to_copy gives a sync-only run a terminal state. Before, instl sync stopped on verifying_downloads and never finished.

The copy phase reports bytes

Planned and done are now the same unit — source bytes. Before, the plan counted wtar items at an expanded estimate while the copy commands reported compressed bytes, so the bar topped out around 77% and then jumped to done. That leaves WTAR_RATIO with no reader, so it goes — flagging it explicitly since it is a documented config var.

copying is announced after folder creation and the pre-copy actions, which contribute no bytes and used to leave the bar at zero.

Unwtar extracts in parallel

Decompression is CPU-bound and was fully serial. DOWNLOAD_PARALLEL_UNWTAR is a kill switch back to the serial path; DOWNLOAD_PARALLEL_WORKERS: 0 means os.cpu_count().

The event channel

downloadEvents is the structured channel Central reads instead of parsing text. It does not replace the legacy progress line — that line must keep being emitted, and does. Redaction is enforced in every helper: hosts may pass, the rest of a URL may not, and local paths are denylisted. DOWNLOAD_TELEMETRY_ENABLED turns the whole channel off without touching the text log.

Scope note: the channel defines five event types and this emits four. The fifth, download.retry_decision, needs the retry policy that produces it, so both it and its tests belong with the download engine rather than here. The docstrings in downloadEvents describe the whole contract including that event, so you will see forward references to a downloadRetry module that arrives in the next PR.

Verification

  • 78 tests across the eight modules this adds or touches, all passing — including a repr/eval round-trip for ReportDownloadState under pybatch/test, per AGENTS.md §8.
  • Thirteen modules run one per process against master3: identical failure sets, zero regressions.
  • instl doit against Central's real Build-index.yaml from this branch: exit 0, and the emitted script py_compiles.
  • The same command run against a master3 worktree produces a structurally identical script — 316 lines both, differing only in paths, timestamp and __INVOCATION_RANDOM_ID__.
  • The two commands this adds to the emitted script were taken from a real install log (a two-platform smoke run) and eval'd against this branch, then round-tripped through repr:
ReportDownloadState(r"copying", reason=r"copy_started", phase_bytes_planned=6135563870, prog_num=842)()
ReportDownloadState(r"completed", reason=r"install_complete", prog_num=3195)()

Not claimed: a green suite. It is not green here and was not green on master3.

Worth pushing back on

  • wtarBatchCommands is the most intrusive change into existing code here. The parallel path is a module-level picklable function that receives only plain data — no command object, no config_vars — and all progress reporting stays on the main process. If you would rather it lived elsewhere, say so.
  • The wire contract is cross-repo. Central consumes this under src/services/instl/behaviours/shell/progress/. schemaVersion stays at 1 because every change here is additive.
  • WTAR_RATIO removal, as above.

This is independent of the housekeeping PR — either order works. The download engine itself (transfer, resume, retry policy, verify) follows separately and builds on this.

An install announced almost nothing between "started" and "done". Central drove
its progress bar by parsing free text, and whole phases - reading the index,
scanning the sync cache, verifying, copying - passed with no signal a UI could
act on. A run that failed emitted nothing terminal at all, so a broken install
and a hung one looked identical from outside.

This adds the reporting, and one real speedup on the way past.

The state machine now has an entry and an exit. preparing is emitted at the top
of the client flow for sync/copy/synccopy, covering the yaml and info-map read,
item calculation and the sync-folder scan. failed is emitted from
PythonBatchRuntime.log_error, so a failed install is no longer indistinguishable
from a hang. ready_to_copy gives a sync-only run a terminal state - before, it
stopped on verifying_downloads and never finished.

The copy phase reports bytes. Planned and done are now the same unit - source
bytes - where before the plan counted wtar items at an expanded estimate while
the copy commands reported compressed bytes, so the bar topped out around 77%
and then jumped. That leaves WTAR_RATIO with no reader, so it goes. copying is
announced after folder creation and the pre-copy actions, which contribute no
bytes and used to sit at zero.

Unwtar extracts across a process pool. Decompression is CPU-bound and was fully
serial. DOWNLOAD_PARALLEL_UNWTAR is a kill switch back to the serial path, and
DOWNLOAD_PARALLEL_WORKERS=0 means os.cpu_count().

downloadEvents is the structured channel Central reads instead of parsing text.
It does not replace the legacy progress line; that line must keep being emitted
and does. Redaction is enforced on every helper: hosts may pass, the rest of a
url may not, and local paths are denylisted. DOWNLOAD_TELEMETRY_ENABLED turns the
whole channel off without touching the text log.

Scope note: the channel defines five event types and this emits four. The fifth,
download.retry_decision, needs the retry policy that produces it, so both it and
its tests belong with the download engine rather than here. The docstrings in
downloadEvents describe the whole contract, including that event.

Verified per module, each in its own process, against master: thirteen modules,
identical failure sets, zero regressions. 78 tests across the eight modules this
adds or touches, all passing, including the client-copy characterization golden
that pins the emitted copy script.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@MatanTiram
MatanTiram requested a review from shaishasag August 2, 2026 21:20
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.

1 participant