Conversation
Do not execute template lifecycle hooks when a DAG or step node is Omitted. Omitted nodes did not run and must not trigger failure hooks. Signed-off-by: Yu Zhou <66656208+zzz-yu@users.noreply.github.com>
👋 PR readiness checkThanks for your contribution! A few automated checks need attention before a maintainer reviews — these are all things you can fix yourself: PR description / templateThe PR description does not appear to follow the template:
(A maintainer may waive this.) Note This PR carries the 🤖 Automated PR-readiness helper — it re-checks each time CI finishes. Unit/E2E test results are not covered here. Questions? See the contributing guide or ask a maintainer. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe lifecycle hook execution path now skips all hooks when the parent node is omitted. This prevents failure hooks from running for omitted nodes. ChangesOmitted node hook handling
Priority: ➖ Normal Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to The omitted-node hook change has no concrete merge-blocking risk identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description explains the issue and the code change, but it does not follow the required template. It omits the required checklist, Motivation, Modifications, Verification, Documentation, and AI sections, and it states that tests were not run.
✨ Finishing Touches🧪 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16977 +/- ##
==========================================
+ Coverage 40.11% 40.16% +0.05%
==========================================
Files 572 572
Lines 44854 44856 +2
==========================================
+ Hits 17991 18016 +25
+ Misses 25068 25050 -18
+ Partials 1795 1790 -5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Fixes #15192
Omitted DAG nodes are not executed and should not run lifecycle hooks. The shared hook executor now returns without evaluating or creating hook nodes for an Omitted parent, preventing failure hooks from firing for tasks whose depends condition was not satisfied.
Tests not run (web editor submission).
Summary by CodeRabbit