-
Notifications
You must be signed in to change notification settings - Fork 165
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
Task status v2 #1558
Task status v2 #1558
Conversation
1ba4b6a
to
13e334d
Compare
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.
LGTM, thanks. I think adding the dedicated task state attribute makes a lot of sense and cleans it up.
One general note just for the record (not sure where else to record, this, maybe we want to put something in the PR description): Because we're updating the task object attributes after the task is scheduled I don't think we won't be able to update the full task data from the worker without clobbering some data there. This is fine because we're not doing this right now, but in the future we'll want to update at least the task status so we should keep that in mind then.
@@ -1197,23 +1186,3 @@ def run(self, evidence, result): | |||
TurbiniaTaskResult object. | |||
""" | |||
raise NotImplementedError | |||
|
|||
def update_task_status(self, task, status=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 we will want to update the task status from the task in order to get mid-run updates, but we can always add this or similar back in when we actually utilize that.
Description of the change
Refactor task status tracking in task_manager to utilize Celery state to determine actual task status.
Applicable issues
Additional information
Checklist