Repository navigation
ci(test): OPS-9045 raise the pre-commit step timeout to 6 minutes - #17532
dmitry-tokarev-nv wants to merge 1 commit into
Conversation
The pre-commit step in Pre Merge had a 3-minute limit. pre-commit/action keys its cache on .pre-commit-config.yaml, so every run of a PR that edits that file starts with an empty hook cache. Such a run spends about 50 s installing hook environments before the hooks start. Two of them timed out after 183 s and 189 s. Runs with a warm cache took up to 159 s. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dmitry Tokarev <dtokarev@nvidia.com>
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Pre-commit timeout .github/workflows/pre-merge.yml |
The pre-commit job timeout increases from 3 minutes to 6 minutes. |
Priority:
Estimated code review effort: 1 (Trivial) | ~5 minutes
Merge Risk: 
3c074
-
The timeout increase addresses cold-cache workflow duration without introducing a concrete correctness, security, or availability risk.
Pre-merge checks: 
-
Check name Status Explanation Title check The title clearly identifies the CI change and the new six-minute pre-commit timeout. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Description check The description includes all required sections and provides clear rationale, changes, validation details, and follow-up work. It links OPS-9045 rather than using the template’s #XXXXformat, and the c…Full details: Description check
-
Explanation
The description includes all required sections and provides clear rationale, changes, validation details, and follow-up work. It links OPS-9045 rather than using the template’s
#XXXXformat, and the checklist remains unchecked; these are non-critical gaps.
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
![dynamo-review-agent[bot]](https://avatars.githubusercontent.com/u/201626793?s=60&v=4){kind=small}
❌ Dynamo PR CI failed — run 40752478609 (attempt 1) on
|
| Other | Jobs |
|---|---|
| Operator Integration | ❌ 1 |
Failure details
1 job failed: the operator cluster test TestClusterFrontendDependencyStartup/nats fails because a container reported Ready while the NATS dependency was supposed to be unavailable (the etcd subtest passed).
❌ Operator Integration: TestClusterFrontendDependencyStartup/nats — container Ready during dependency outage (line 240)
Job: Operator Integration · Failed step: Run operator integration tests · Logs: gh run view --job 123047479384 -R ai-dynamo/dynamo --log-failed
dgd_dependency_startup_clustertest_test.go:225: Keep the dependency unavailable for 60 seconds while both original pods stay unready without restarting
dgd_dependency_startup_clustertest_test.go:240:
Error Trace: .../deploy/operator/internal/controller/dgd_dependency_startup_clustertest_test.go:240
Error: Should be false
Test: TestClusterFrontendDependencyStartup/nats
--- FAIL: TestClusterFrontendDependencyStartup (140.13s)
--- FAIL: TestClusterFrontendDependencyStartup/nats (26.56s)
--- PASS: TestClusterFrontendDependencyStartup/etcd (113.57s)
FAIL github.com/ai-dynamo/dynamo/deploy/operator/internal/controller 353.966s
make[1]: *** [Makefile:375: clustertest] Error 1
##[error]Process completed with exit code 2.
Line 240 is require.False(t, status.Ready): during the 60-second window where the NATS dependency Service has no endpoints, one of the frontend/worker containers became Ready after ~26s, so the test failed instead of seeing both pods stay unready. All other operator cluster tests in the package passed; the ReconcileError ... object has been modified events in the log are optimistic-concurrency noise, not the cause.
For agents
{"pr": 17532, "run_id": 40752478609, "run_attempt": 1, "head_sha": "3c074d3f43a8ecac4825f055548159be9b04b69a",
"failures": [{"job": "Operator Integration", "job_id": 123047479384, "failed_step": "Run operator integration tests", "signature": "TestClusterFrontendDependencyStartup/nats: container Ready while NATS unavailable (require.False(status.Ready) at dgd_dependency_startup_clustertest_test.go:240)", "tests": ["deploy/operator/internal/controller::TestClusterFrontendDependencyStartup/nats"], "log_cmd": "gh run view --job 123047479384 -R ai-dynamo/dynamo --log-failed"}]}Posted automatically by Devin for run 40752478609. Updated on every full-CI run of this PR.
|
closing since #17536 reduced pre-commit runtime. NExt step removing black from pre-commit will reduce it even further |
Linear: OPS-9045
Overview
The
pre-commitjob in Pre Merge timed out twice on 2026-10-28, on PRs that edit.pre-commit-config.yaml. Those runs start with an empty hook cache and need more than the 3 minutes that the step allows. This PR raises the step limit to 6 minutes.Key Changes
Core logic: 1 line.
.github/workflows/pre-merge.yml:timeout-minuteson thepre-commit/actionstep goes from 3 to 6, with a one-line comment.Validation
The two timeouts:
Cache not foundThe action has timed out.Cache not foundThe action has timed out.Cause:
pre-commit/actionkeys its cache on the hash of.pre-commit-config.yaml. Both PRs edit that file, so each of their runs misses the cache. Their logs show 52 to 56 s of hook environment installs before the first hook result. With the cache of main, the same hooks took 119 s, 120 s, and 159 s.Headroom on main: across the 333 successful
pre-commitjobs of 2026-10-28, the job time was median 138 s, p95 165 s, and max 196 s. These times include 10 to 15 s of checkout and setup outside the step limit. Four hooks use most of the step time:black(47 to 55 s),flake8(23 to 27 s), the pytest marker report (17 to 19 s), andcheck yaml(11 to 13 s).With a 6-minute limit, the slowest cold run (189 s) uses about half of the time. The job has no job-level
timeout-minutes, so the step limit is the only limit.pre-commit run --all-filespassed locally, and actionlint 1.7.12 reports the same two existing notes for the base file and this PR.This PR does not edit
.pre-commit-config.yaml, so its ownpre-commitrun uses the warm cache and does not reproduce a cold run. That job 123042520368 passed in 106 s, with 84 s in the pre-commit step, which is 23% of the new limit.Issues and Ongoing Work
black,isort, andflake8to ruff. Those three hooks take 75 to 90 s, and ruff does the same work in about 1 s. Theflake8hook enforces nothing today. YAML splits its flow-styleargslist at every comma, so flake8 receives--select=Calone. With those arguments, flake8 exits 0 on main and misses a planted syntax error.restore-keys, so that a cold run reuses the environments that main cached. It is not in this PR.Pull Request Checklist
/pr-checklistskill.🤖 Generated with Claude Code