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

Workflow cancel , restart, resume as admin API #187

Merged
merged 15 commits into from
Jan 29, 2025
Merged

Conversation

manojdbos
Copy link
Contributor

No description provided.

@manojdbos manojdbos marked this pull request as ready for review January 28, 2025 23:30
else:
new_wfUuid = output[0].workflowUUID

print(new_wfUuid)
Copy link
Member

Choose a reason for hiding this comment

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

Let's remove the prints

@@ -767,13 +777,30 @@ def execute_workflow_id(cls, workflow_id: str) -> WorkflowHandle[Any]:
"""Execute a workflow by ID (for recovery)."""
return execute_workflow_by_id(_get_dbos_instance(), workflow_id)

@classmethod
def restart_workflow(cls, workflow_id: str) -> None:
Copy link
Member

@kraftp kraftp Jan 29, 2025

Choose a reason for hiding this comment

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

Are these new methods part of the public API?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are called from the admin server. Not sure what you mean by public api

dbos/cli.py Outdated
print(f"Failed to resume workflow {uuid}. Status code: {response.status_code}")


@workflow.command(help="Restart a workflow that has been cancelled with a new id")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@workflow.command(help="Restart a workflow that has been cancelled with a new id")
@workflow.command(help="Restart a workflow from the beginning with a new workflow ID")

Copy link
Member

@kraftp kraftp 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!

Comment on lines +19 to +21
# /workflows/:workflow_id/cancel
# /workflows/:workflow_id/resume
# /workflows/:workflow_id/restart
Copy link
Contributor

Choose a reason for hiding this comment

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

What are these comments for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For other engineers to see what the url looks like

def _handle_restart(self, workflow_id: str) -> None:
self.dbos.restart_workflow(workflow_id)
print("Restarting workflow", workflow_id)
self.send_response(200)
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 return 204? The server doesn't return anything to the client

dbos/_dbos.py Outdated

@classmethod
def resume_workflow(cls, workflow_id: str) -> None:
"""Cancel a workflow by ID."""
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"""Cancel a workflow by ID."""
"""Resume a workflow by ID."""

dbos/_dbos.py Outdated
@@ -231,9 +233,17 @@ def __new__(
f"DBOS configured multiple times with conflicting information"
)
config = _dbos_global_registry.config

print("calling new setting up the global instance")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to call the DBOS logger for all these statements? So they get exported, etc?

@manojdbos manojdbos merged commit 983a028 into main Jan 29, 2025
5 checks passed
@manojdbos manojdbos deleted the workflows_restart branch January 29, 2025 17:45
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.

3 participants