Uh oh!
There was an error while loading. Please reload this page.
Add new --follow mode for tower apps logs with resilient streaming and tests - #171
Conversation
bradhe
commented
Jan 16, 2026
Nice thanks @burakdede, we should make the default branch in this repo I'll review the content now.! |
There was a problem hiding this comment.
Besides the failure re: linting, this is generally really good! I'd really love to merge this with the following logic in the do_run function in crates/tower-cmd/src/run.rs -- but I can manage that on our side! All the stuff around retries, etc., is really good and missing from the other implementation.
I'll leave this open for a bit just in case you decide this is something you want to tackle, but otherwise I'll rebase this against develop and land this over the weekend!
| if is_run_finished(&run) { | ||
| if let Ok(resp) = api::describe_run_logs(&config, &name, seq).await { | ||
| for line in resp.log_lines { | ||
| emit_log_if_new(&line, &mut last_line_num); | ||
| } | ||
| } | ||
| return; | ||
| } |
There was a problem hiding this comment.
This is good, we really should unify these endpoints at some point to make this easier 🙃
| let (cancel_tx, cancel_rx) = oneshot::channel(); | ||
| cancel_monitor = Some(cancel_tx); | ||
| let run_complete = monitor_run_completion(&config, &name, seq, cancel_rx); | ||
| match api::stream_run_logs(&config, &name, seq).await { |
There was a problem hiding this comment.
This should be fine functionally, but comparing it to our implementation for starting a remote run and following the logs, we wait for the run to start first...that may not be necessary, we should be able to just start listening? I'll double check that.
There was a problem hiding this comment.
I went ahead and added a small start‑state wait before opening the log stream to mirror the remote‑run flow, since we don’t have clear backend guarantees that pre‑start streaming is valid.
So the states aren’t consistent: in apps logs --follow, terminal states are not errors; in run, they are. That behavior difference might be fine because apps logs --follow is designed to show logs for completed runs, while run is about waiting for a live run to begin.
Feel free to cherry‑pick after you double‑check backend behavior.
- deduplicate log lines across reconnects / might be an issue - add unit test for deduplication and out of order logs
518721d to
709d13eComparebradhe
commented
Jan 18, 2026
Awesome, thanks @burakdede really appreciate it! |
Uh oh!
There was an error while loading. Please reload this page.
* Revert "fix(schedules): restore direct update semantics and strengthen parsing tests (#205)" This reverts commit 58456c5. * Revert "Set app description correctly on create only (#182)" This reverts commit a3307b0. * Revert "Add new `--follow` mode for `tower apps logs` with resilient streaming and tests (#171)" This reverts commit fd79baf. * Revert "fix(cli-help): clarify usage/examples for apps, schedules, secrets (#169)" This reverts commit 7f6d0a7. * Revert "Fix: better handling of non-iterable dbt result objects and selectable dbt commands (#143)" This reverts commit 6239423. * Add --follow mode to tower apps logs * Add positional schedule id handling * Make app description create-only * Improve subcommand help text * Handle non-iterable dbt results and selector targeting * Add integration coverage for run cancel, follow mode, and app descriptions * Apply black formatting * Fix warning handling in follow mode and stream error formatting
Addresses improvement required #164
--followtotower apps logswith SSE streaming, retry/backoff, Ctrl+C handling, dedupe across reconnects, and fatal‑error exits.Tests should cover most of the critical aspects of the implementation but I have also simulated some tests via local
towerbuild and deploying app for;Broken pipeand CLI panic #162 still fails when piping to head (known issue).Drain Logs & Clean Exit
Intermittent Failure Results & Slightly-Graceful Exit
Kill Log Stream & Clean Exit