Skip to content

cleanup_dead_jobs() , active jobs, empty case not handled#277

Closed
smallzhao wants to merge 2 commits into
wooey:masterfrom
smallzhao:master
Closed

cleanup_dead_jobs() , active jobs, empty case not handled#277
smallzhao wants to merge 2 commits into
wooey:masterfrom
smallzhao:master

Conversation

@smallzhao
Copy link
Copy Markdown

issue #261

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.1%) to 77.221% when pulling a723a2a on smallzhao:master into e9e9dfd on wooey:master.

Comment thread wooey/tasks.py
inspect = celery_app.control.inspect()
active_tasks = {task['id'] for worker, tasks in six.iteritems(inspect.active()) for task in tasks}
task_infos = inspect.active()
if task_infos == None:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It might be worth having this line as if task_infos is None: according to https://stackoverflow.com/questions/14247373/python-none-comparison-should-i-use-is-or

Copy link
Copy Markdown
Member

@Chris7 Chris7 left a comment

Choose a reason for hiding this comment

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

Thank you for taking this on! I have one comment about a better name and simplifying the None check.

Comment thread wooey/tasks.py
# Get active tasks from Celery
inspect = celery_app.control.inspect()
active_tasks = {task['id'] for worker, tasks in six.iteritems(inspect.active()) for task in tasks}
task_infos = inspect.active()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you call this worker_info? I think you can also achieve the None check here by doing:
worker_info = inspect.active() or {}

@Chris7
Copy link
Copy Markdown
Member

Chris7 commented Jun 9, 2019

Thanks for this patch, I implemented it in #281

@Chris7 Chris7 closed this Jun 9, 2019
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.

4 participants