-
Notifications
You must be signed in to change notification settings - Fork 3
#685 Fix when transformers and sinks are marked as not allowed to run in parallel have the behavior of transformations with self-dependencies. #688
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… in parallel have the behavior of transformations with self-dependencies.
WalkthroughIntroduces Changes
Sequence DiagramsequenceDiagram
participant TaskRunner as Task Runner
participant Task
participant Job
participant NextTask as Next Task
TaskRunner->>Task: Execute task
Task-->>TaskRunner: Fails
TaskRunner->>Job: Check isSelfDependent
alt Job is self-dependent
Job-->>TaskRunner: true
TaskRunner->>NextTask: Skip task
Note over TaskRunner: Previous failure + self-dependent<br/>→ Stop execution chain
else Job is not self-dependent
Job-->>TaskRunner: false
TaskRunner->>NextTask: Execute task
Note over TaskRunner: Previous failure but not self-dependent<br/>→ Continue execution chain
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@pramen/core/src/main/scala/za/co/absa/pramen/core/pipeline/Job.scala`:
- Line 45: The new abstract member isSelfDependent on trait Job breaks
downstream implementations; provide a compatibility-safe default by making
isSelfDependent a concrete method returning false in the Job trait (i.e., change
the declaration in Job so implementations keep working while allowing
overrides), and update any internal implementers that need true to explicitly
override isSelfDependent.
|
|
||
| def allowRunningTasksInParallel: Boolean | ||
|
|
||
| def isSelfDependent: Boolean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid breaking external Job implementations with new abstract API.
Adding a new abstract member to a public trait is binary/source breaking for downstream implementers that aren’t updated/recompiled. Consider providing a safe default (e.g., false) or explicitly documenting the required migration in release notes. Line 45.
💡 Suggested compatibility-friendly default
- def isSelfDependent: Boolean
+ def isSelfDependent: Boolean = falseTo verify in-repo implementers:
#!/bin/bash
# Find Job implementations that may need the new member.
rg -nP --type=scala '\bextends\s+Job\b|\bwith\s+Job\b'🤖 Prompt for AI Agents
In `@pramen/core/src/main/scala/za/co/absa/pramen/core/pipeline/Job.scala` at line
45, The new abstract member isSelfDependent on trait Job breaks downstream
implementations; provide a compatibility-safe default by making isSelfDependent
a concrete method returning false in the Job trait (i.e., change the declaration
in Job so implementations keep working while allowing overrides), and update any
internal implementers that need true to explicitly override isSelfDependent.
Unit Test Coverage
Files
|
Closes #685
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.