Conversation
📝 WalkthroughWalkthroughThis PR enables DAG-level automatic retries to be disabled by allowing Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/cmn/schema/dag.schema.json (1)
890-895:⚠️ Potential issue | 🟡 MinorKeep step retry-policy wording step-specific.
stepRetryPolicyis used fordefaults.retry_policyand per-stepretry_policy, so this description currently tells users that a step retry limit controls DAG-level scheduler retries. That can mislead schema-driven docs/editors.📝 Proposed wording fix
"limit": { "oneOf": [ { "type": "integer" }, { "type": "string" } ], - "description": "Maximum number of scheduler-issued DAG retry attempts. Use 0 to disable DAG-level automatic retries." + "description": "Maximum number of retry attempts for a failed step." },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cmn/schema/dag.schema.json` around lines 890 - 895, The description for the "limit" property currently implies DAG-level scheduler retry behavior; update it to be step-specific by clarifying that this is the maximum number of retry attempts for an individual step's retry policy (used by defaults.retry_policy and per-step retry_policy), and note that a value of 0 disables automatic retries for that step—refer to the "limit" property inside stepRetryPolicy to locate the text to change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@internal/cmn/schema/dag.schema.json`:
- Around line 890-895: The description for the "limit" property currently
implies DAG-level scheduler retry behavior; update it to be step-specific by
clarifying that this is the maximum number of retry attempts for an individual
step's retry policy (used by defaults.retry_policy and per-step retry_policy),
and note that a value of 0 disables automatic retries for that step—refer to the
"limit" property inside stepRetryPolicy to locate the text to change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0758face-aeec-49d8-a42c-2e62e59a0a8c
📒 Files selected for processing (8)
internal/cmn/schema/dag.schema.jsoninternal/cmn/schema/dag_schema_test.gointernal/core/exec/runstatus_test.gointernal/core/spec/dag.gointernal/core/spec/loader.gointernal/core/spec/loader_test.gointernal/service/scheduler/retry_scanner_test.goskills/dagu/references/schema.md
Summary
retry_policy.limit: 0to disable scheduler-issued DAG retrieslimit: 0Closes #2016
User-facing docs
dagucloud/docsonmain:d03c0b6 docs: clarify disabled DAG retriesTesting
go test ./internal/core/spec ./internal/cmn/schema ./internal/core/exec ./internal/service/scheduler -count=1go test ./internal/cmn/schema -count=1go test ./internal/intg/queue -run 'TestSchedulerRetryScanner/DisabledByChildSkipsInheritedBaseRetryPolicy' -count=1go test ./internal/intg/queue -run TestSchedulerRetryScanner -count=1pnpm buildin./docsgit diff --checkin the main repo and./docsSummary by CodeRabbit
Release Notes
New Features
limit: 0in the retry policy configuration.Improvements
Documentation
limit: 0.