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

rest: set workflow status to pending after starting the workflow #371

Merged
merged 1 commit into from
Apr 28, 2021

Conversation

audrium
Copy link
Member

@audrium audrium commented Apr 21, 2021

closes #363

Depends on reanahub/reana-db#123 (tests fail because pending status is not present yet)

Changes

  • Sets workflow status to pending right after the workflow has been started

  • Adds get_workflow_status_name() function to return back the workflow status

    • pending status should be transparent to the user, therefore in that case the queued status is returned
  • Modifies get_workflows() endpoint to include pending workflows if filtered by queued status

How to test

I've tested it by running simple helloworld demo and modifying reana_workflow_engine_serial by adding

import time
time.sleep(20)

just before publish_workflow_start(). In this case we can simulate that workflow is stuck a bit in pending state.

In addition these lines could be commented to inspect the pending status using cli (note that UI will break, since the new state is not supported there)

@audrium audrium force-pushed the 366-new-pending-state branch 3 times, most recently from 5d6f418 to 0f248dc Compare April 22, 2021 08:46
@audrium audrium marked this pull request as ready for review April 22, 2021 11:35
@@ -125,6 +127,18 @@ def get_workflow_name(workflow):
return workflow.name + "." + str(workflow.run_number)


def get_workflow_status_name(workflow):
Copy link
Member

Choose a reason for hiding this comment

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

Have you tried to move such logic into a new property in REANA-DB? (asking just in case you already found a blocker 😄 ). We could have something like Workflow.status_ui (this name is bad, this is just a placeholder) which could be a hybrid property. This way we could perhaps overwrite the .in_ method for that property (useful for filtering), and also we wouldn't break the UI nor the client (this would access hypothetical Workflow.status_ui).

On the one hand, if we do such a split I feel it should be close to the models, on the other hand, I see it might be confusing to decide which one to use. Let me know what you think.

Copy link
Member Author

Choose a reason for hiding this comment

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

Created issue to follow up on this here

@audrium audrium force-pushed the 366-new-pending-state branch 2 times, most recently from dccd29a to 8d494f3 Compare April 27, 2021 07:11
@audrium audrium force-pushed the 366-new-pending-state branch from 8d494f3 to ae4f10e Compare April 28, 2021 13:04
@diegodelemos diegodelemos merged commit ae4f10e into reanahub:master Apr 28, 2021
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.

workflow-run-manager: create new pending status
2 participants