-
Notifications
You must be signed in to change notification settings - Fork 13
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
add stop/delete experiment endpoint #13
Conversation
- track list of celery task ids and cancel running tasks to free up the space
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 good! Just added a few remarks.
api/api/experiments.py
Outdated
@@ -149,9 +150,11 @@ class Experiment(sqlm.SQLModel, table=True): # type: ignore[call-arg] | |||
runner: runners.Runner = sqlm.Relationship(back_populates="experiments") | |||
sink_id: int = sqlm.Field(foreign_key="sink.id") | |||
sink: sinks.Sink = sqlm.Relationship(back_populates="experiments") | |||
task_ids: str | None = sqlm.Field(default=None) |
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 a cleaner way than using str
is to store the IDs in a list; see here
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.
thanks, i was looking for that.
I tried task_ids: List[str]
and variations of that but was not working.
if not exp: | ||
raise HTTPException(status_code=404, detail="Experiment not found") | ||
for task_id in exp.task_ids.split(","): | ||
RUNNER.control.revoke(task_id, terminate=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.
Nice!
|
||
|
||
@router.delete("/{experiment_id}") | ||
def delete_experiment(experiment_id: int): |
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.
Do the tasks need to be stopped if the experiment is deleted?
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.
yes! good catch
when deleting experiment also stop the corresponding training and inference tasks from running
feature pr: add additional way of running training and inference jobs that are infinite in its nature