-
Notifications
You must be signed in to change notification settings - Fork 3
Aileen Dashboard Improvements Vers 1 #135
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good! Sprinkled some comments throughout. You should also run pre-commit run --all-files
locally and fix the handful of things that need addressing (see https://pre-commit.com/)
|
||
def resume_workflow_run(wf_run: WorkflowRun) -> None: | ||
"""Resumes the workflow run""" | ||
wf_run.status = WorkflowStatus.IN_PROGRESS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it makes sense to set the status as IN_PROGRESS here, because the workflow may or may not resume running immediately, depending on the module. But we should have some logic in the scheduler to then update the state to RUNNING if the module does resume.
@router.api_route("/reset_wf/{wf_run_id}", methods=["POST"]) | ||
def reset_workflow(wf_run_id: str) -> None: | ||
"""Restarts a workflow""" | ||
state_manager.paused = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I understand why we're wrapping the commands by pausing/unpausing the workcell. Should the state manager lock be acquired out here instead of in the workflow.py methods?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be more clear: this seems dangerous because you can get into race conditions where multiple admin commands being sent can cause the workcell to pause/unpause mid-admin action
Events tab and display, content filtering, admin actions for workflows (Cancel, Pause/Resume, Reset).