fix(smoke-tests): don't let a log-fetch timeout mask the real failure - #129
Open
spal1 wants to merge 1 commit into
Open
fix(smoke-tests): don't let a log-fetch timeout mask the real failure#129spal1 wants to merge 1 commit into
spal1 wants to merge 1 commit into
Conversation
When a training job fails, poll_job_status dumps its logs via
`truss train logs` for debugging. That call is best-effort (check=False),
but check=False only covers a nonzero exit: subprocess.run raises
TimeoutExpired, which propagated out of poll_job_status and aborted main
before the summary was written.
The result is that every failing run reports only
subprocess.TimeoutExpired: Command '[... 'train', 'logs' ...]'
timed out after 300 seconds
and never the job status or error it had already fetched. The smoke tests
have been red since 2026-06-23 and none of those runs named the cause;
it took a manual Loki query to find it (an HF weights download 403).
Catch TimeoutExpired in run_truss_cli: echo whatever the command emitted,
then either raise a clear RuntimeError (check=True, an operation the test
depends on) or return a synthetic CompletedProcess with exit 124
(check=False, a best-effort dump) so the caller reports the real outcome.
Give the two log dumps a 120s budget instead of 300s, since they are
diagnostics rather than something the test depends on.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Makes the smoke test report the actual training failure instead of a timeout from the log-dump command that runs afterward.
Why
When a job reaches a terminal failure,
poll_job_statusdumps its logs viatruss train logsfor debugging. That call is best-effort (check=False), butcheck=Falseonly covers a nonzero exit —subprocess.runraisesTimeoutExpired, which propagated out ofpoll_job_statusand abortedmainbefore the summary was written.So every failing run reports only this:
and never the
TRAINING_JOB_FAILEDstatus and error message it had already successfully fetched one line earlier.These smoke tests have been red on every scheduled run since 2026-06-23 (last green on
main: run 28016797245), and not one of those runs named the cause. Finding it required querying Loki by hand: the training container dies withOSError: Can't load the model for 'Qwen/Qwen3-0.6B'after a403 Forbiddenfrom the HF Xet CDN onmodel.safetensors. Same failure on both accelerators and across two clusters (neb-eunorth1-prod-1, ori-dfw-prod-1).This PR does not fix that 403 — it makes the next occurrence of any failure self-diagnosing.
How
run_truss_clicatchessubprocess.TimeoutExpired, echoes whatever the command emitted before hanging, and then either raises a clearRuntimeError(check=True, an operation the test depends on) or returns a syntheticCompletedProcesswith exit 124 (check=False, a best-effort dump) so the caller carries on and reports the real outcome._echo_cli_outputso the timeout path prints partial output the same way the normal path does.timeout=LOG_FETCH_TIMEOUT(120s) rather than the default 300s, since they are diagnostics rather than something the test depends on. This also cuts ~3 minutes off each failing run.Testing
Exercised both branches by stubbing
subprocess.runto raiseTimeoutExpired:I have not run the full smoke test against a live remote; that needs the workflow's credentials.
Related
pyproject.tomlmoves off thetruss==0.18.16pin, which is why this harness-side guard is worth having on its own.