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

tweaks to not use celery AsyncResult and EagerResult for task status #89

Merged
merged 2 commits into from
Dec 19, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions smartmin/csv_imports/migrations/0003_importtask_task_status.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# -*- coding: utf-8 -*-
from __future__ import unicode_literals

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('csv_imports', '0002_auto_20161118_1920'),
]

operations = [
migrations.AddField(
model_name='importtask',
name='task_status',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can probably just be status, no need to repeat task here.

field=models.CharField(default='PENDING', max_length=32),
),
]
28 changes: 14 additions & 14 deletions smartmin/csv_imports/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@ def generate_file_path(instance, filename):


class ImportTask(SmartModel):
PENDING = 'PENDING'
STARTED = 'STARTED'
RUNNING = 'RUNNING'
SUCCESS = 'SUCCESS'
FAILURE = 'FAILURE'
Copy link
Collaborator

Choose a reason for hiding this comment

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

COMPLETED and FAILED


READY_STATES = [SUCCESS, FAILURE]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe FINAL_STATES here? Ready seems a bit ambiguous. Also why not use a single character here? Maybe change SUCCESS to COMPLETE so there isn't overlap with S

Copy link
Collaborator

Choose a reason for hiding this comment

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

Just realized you might be using these to keep compatibility with what the Celery states are. If so then that's fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right I got those from Celery even if it is not guaranteed that the will always be in sync.
Also for task_status I wanted to keep the status method for backward compatibility in case it was called by someone

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, fair enough and a good reason. Looks good then, merge at will.


csv_file = models.FileField(upload_to=generate_file_path, verbose_name="Import file",
help_text="A comma delimited file of records to import")

Expand All @@ -32,31 +40,23 @@ class ImportTask(SmartModel):

task_id = models.CharField(null=True, max_length=64)

task_status = models.CharField(max_length=32, default=PENDING)

def start(self):
from .tasks import csv_import
self.log("Queued import at %s" % timezone.now())
self.save(update_fields=['import_log'])
self.task_status = self.STARTED
self.save(update_fields=['import_log', 'task_status'])
result = csv_import.delay(self.pk)
self.task_id = result.task_id
self.save(update_fields=['task_id'])

def done(self):
if self.task_id:
if getattr(settings, 'CELERY_ALWAYS_EAGER', False):
result = EagerResult(self.task_id, None, 'SUCCESS')
else:
result = AsyncResult(self.task_id)
return result.ready()
return self.task_status in self.READY_STATES

def status(self):
status = "PENDING"
if self.task_id:
if getattr(settings, 'CELERY_ALWAYS_EAGER', False):
result = EagerResult(self.task_id, None, 'SUCCESS')
else:
result = AsyncResult(self.task_id)
status = result.state
return status
return self.task_status

def log(self, message):
self.import_log += "%s\n" % message
Expand Down
33 changes: 19 additions & 14 deletions smartmin/csv_imports/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,31 +15,36 @@

@task(track_started=True)
def csv_import(task_id): # pragma: no cover
task = ImportTask.objects.get(pk=task_id)
task_obj = ImportTask.objects.get(pk=task_id)
log = StringIO()

task.task_id = csv_import.request.id
task.log("Started import at %s" % timezone.now())
task.log("--------------------------------")
task.save()
task_obj.task_id = csv_import.request.id
task_obj.task_status = ImportTask.RUNNING
task_obj.log("Started import at %s" % timezone.now())
task_obj.log("--------------------------------")
task_obj.save()

try:
with transaction.atomic():
model = class_from_string(task.model_class)
records = model.import_csv(task, log)
task.save()
model = class_from_string(task_obj.model_class)
records = model.import_csv(task_obj, log)
task_obj.task_status = ImportTask.SUCCESS
task_obj.save()

task.log(log.getvalue())
task.log("Import finished at %s" % timezone.now())
task.log("%d record(s) added." % len(records))
task_obj.log(log.getvalue())
task_obj.log("Import finished at %s" % timezone.now())
task_obj.log("%d record(s) added." % len(records))

except Exception as e:
import traceback
traceback.print_exc(e)

task.log("\nError: %s\n" % e)
task.log(log.getvalue())
task_obj.task_status = ImportTask.FAILURE

task_obj.log("\nError: %s\n" % e)
task_obj.log(log.getvalue())
task_obj.save()

raise e

return task
return task_obj