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

Mark Workflow as FAILED if the action inside the workflow fails instead of marking it as COMPLETE. #7546

Merged
merged 1 commit into from
Feb 2, 2017

Conversation

sagarkapare
Copy link
Contributor

JIRA: https://issues.cask.co/browse/CDAP-6008
Build: http://builds.cask.co/browse/CDAP-DUT5245-15

Fix contains -

  1. Moving recording of the RunRecord of the Workflow to the WorkflowProgramRunner
  2. Added 2 methods in the ProgramManager interface - waitForRun, and waitForRuns which are used in the unit tests instead of waitForFinish to make sure that the Workflow program is reached to the specified state.

@prinam prinam added the 4.1 label Jan 7, 2017
@sagarkapare
Copy link
Contributor Author

Build is green here - http://builds.cask.co/browse/CDAP-DUT5245-17
@chtyim Please take a look. thanks!

@prinam prinam requested review from chtyim and anwar6953 and removed request for chtyim January 27, 2017 02:03

@Override
public void completed() {
LOG.debug("Program {} completed successfully.", program.getId());
Copy link
Contributor

Choose a reason for hiding this comment

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

can we log the runid also

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.


@Override
public void killed() {
LOG.debug("Program {} killed.", program.getId());
Copy link
Contributor

Choose a reason for hiding this comment

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

can we log the runid also

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.

runtimeStore.setStart(program.getId(), runId.getId(), startTimeInSeconds, twillRunId,
options.getUserArguments().asMap(), options.getArguments().asMap());

switch (state) {
Copy link
Contributor

Choose a reason for hiding this comment

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

what about KILLED

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually no need to have KILLED state here, since when actions such as SUSPEND/KILLED would happen init listener would have executed at that time. This is similar to the https://github.com/caskdata/cdap/blob/develop/cdap-app-fabric/src/main/java/co/cask/cdap/internal/app/services/ProgramLifecycleService.java#L341

@anwar6953
Copy link
Contributor

LGTM

@sagarkapare
Copy link
Contributor Author

Build running here - https://builds.cask.co/browse/CDAP-DUT5389-1

@sagarkapare
Copy link
Contributor Author

Thank you @anwar6953 for the review. Build is green. Squashing and merging.

@sagarkapare sagarkapare merged commit 92d5dd4 into develop Feb 2, 2017
@sagarkapare sagarkapare deleted the fix/cdap-6008 branch February 2, 2017 02:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants