Skip to content
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

vdk-core: add ability to cancel remaining job steps #1188

Merged
merged 5 commits into from
Sep 26, 2022

Conversation

mrMoZ1
Copy link
Contributor

@mrMoZ1 mrMoZ1 commented Sep 23, 2022

what: Added the ability to cancel the remaining job/template steps through the job_input interface.

why: This feature is required by users in certain cases e.g: if a data job depends on processing data from a source which has indicated no new entries since last run, then we can skip the execution.

testing: Tested locally with data job/templates and added functional tests.

Signed-off-by: Momchil Zhivkov [email protected]

@antoniivanov
Copy link
Collaborator

I think this is a cool new feature. I've talked with some users in the past who have complained they cannot stop a job in the middle. We should popularize it after releasing it. Thanks for coming up with it.

Signed-off-by: mrMoZ1 <[email protected]>
@mrMoZ1 mrMoZ1 requested a review from antoniivanov September 26, 2022 14:51
Signed-off-by: mrMoZ1 <[email protected]>
Copy link
Collaborator

@antoniivanov antoniivanov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Just a few final touches around consistent naming and more logs can be added.,

Signed-off-by: mrMoZ1 <[email protected]>
@mrMoZ1 mrMoZ1 enabled auto-merge (squash) September 26, 2022 15:19
@mrMoZ1 mrMoZ1 merged commit 18a62fd into main Sep 26, 2022
@mrMoZ1 mrMoZ1 deleted the person/mzhivkov/impala-template-regression branch September 26, 2022 15:24
@@ -146,7 +146,7 @@ def invoke_run_function(func: Callable, job_input: IJobInput, step_name: str):
what_happened=f"Data Job step {step_name} completed with error.",
why_it_happened=errors.MSG_WHY_FROM_EXCEPTION(e),
consequences="I will not process the remaining steps (if any), "
"and this Data Job execution will be marked as failed.",
Copy link
Collaborator

@antoniivanov antoniivanov Sep 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't notice this. That's why I dislike the exception approach for control flow (as i mentioned before).

Isn't it better to catch teh exception and handle it, re-raise it. I think this will print an error in the logs - but we have not error (and that may confure users)

mrMoZ1 pushed a commit that referenced this pull request Sep 27, 2022
what: The template arguments validator now makes use of the skip_remaining_steps() method when an empty source view is returned.

why: Users of the template requested that empty source view stopped resulting in a User Error, after a discussion it was agreed that this is the correct and desired behaviour.

testing: this change depends on #1188
Edited failing regression tests.

Signed-off-by: Momchil Zhivkov [email protected]
antoniivanov pushed a commit that referenced this pull request Sep 29, 2022
what: Added the ability to cancel the remaining job/template steps through the job_input interface.

why: This feature is required by users in certain cases e.g: if a data job depends on processing data from a source which has indicated no new entries since last run, then we can skip the execution.

testing: Tested locally with data job/templates and added functional tests.

Signed-off-by: Momchil Zhivkov [email protected]
antoniivanov pushed a commit that referenced this pull request Sep 29, 2022
what: The template arguments validator now makes use of the skip_remaining_steps() method when an empty source view is returned.

why: Users of the template requested that empty source view stopped resulting in a User Error, after a discussion it was agreed that this is the correct and desired behaviour.

testing: this change depends on #1188
Edited failing regression tests.

Signed-off-by: Momchil Zhivkov [email protected]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants