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

fix: mark run as done when skipif is true #1672

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

saranyailla
Copy link
Member

@saranyailla saranyailla commented Nov 23, 2024

Issue #, if available:

Description of changes:
Currently, when skipif evaluates to true, RunStatus is set to OK in the result. But, RunStatus.OK is currently not handled in by the Run handler. We could either add a case to handle OK or mark the skipif RunStatus to NothingDone.

This doesn't impact INSTALL lifecycle as it only cares when RunStatus is Errored.

if (runResult.getRunStatus() == RunStatus.NothingDone) {
reportState(State.FINISHED);
logger.atInfo().setEventType("generic-service-finished").log("Nothing done");
return;
} else if (runResult.getRunStatus() == RunStatus.Errored) {
if (runResult.getStatusCode() == null) {
serviceErrored("Script errored in run");
} else {
serviceErrored(runResult.getStatusCode(), "Script errored in run");
}
return;
} else if (runResult.getExec() != null) {
reportState(State.RUNNING);
updateSystemResourceLimits();
systemResourceController.addComponentProcess(this, runResult.getExec().getProcess());
}

Run lifecyle doesn't handle
Why is this change necessary:

How was this change tested:

  • Updated or added new unit tests.
  • Updated or added new integration tests.
  • Updated or added new end-to-end tests.
  • If my code makes a remote network call, it was tested with a proxy.

Any additional information or context required to review the change:

Documentation Checklist:

  • Updated the README if applicable.

Compatibility Checklist:

  • I confirm that the change is backwards compatible.
  • Any modification or deletion of public interfaces does not impact other plugin components.
  • For external library version updates, I have reviewed its change logs and Nucleus does not consume
    any deprecated method or type.

Refer to Compatibility Guidelines for more information.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@saranyailla saranyailla force-pushed the skipif-run branch 5 times, most recently from 4ee78a3 to 8629bb1 Compare November 23, 2024 03:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant