From 7de02a87519cbbdc5071ed6d92f259fc31c85470 Mon Sep 17 00:00:00 2001 From: Emil Melnikov Date: Wed, 22 Jan 2020 10:07:59 +0100 Subject: [PATCH 1/4] Add started and finished times to Job * Add "started_on" and "finished_on" fields to the Job model. * Add the "Running Time" column to the project job list view. Other changes: * Remove the row number field from the project job list view: job ID should be sufficient for debugging. * Add full job ID tooltip. * Change submission time tooltip from short human-readable format to full date string in order to provide more information. --- batch/admin.py | 4 +-- batch/migrations/0007_auto_20200122_0827.py | 23 +++++++++++++++++ batch/models.py | 22 +++++++++++++++- batch/serializers.py | 2 +- .../templates/batch/project_detail.html | 25 +++++++++++++------ 5 files changed, 65 insertions(+), 11 deletions(-) create mode 100644 batch/migrations/0007_auto_20200122_0827.py diff --git a/batch/admin.py b/batch/admin.py index ea60e27..640ca2c 100644 --- a/batch/admin.py +++ b/batch/admin.py @@ -11,7 +11,7 @@ class ProjectAdmin(admin.ModelAdmin): @admin.register(models.Job) class JobAdmin(admin.ModelAdmin): list_display = ["id", "owner", "status", "project", "external_id", "created_on", "updated_on"] - readonly_fields = ["owner", "project", "external_id", "created_on", "updated_on"] - list_filter = ["status", "created_on"] + readonly_fields = ["owner", "project", "external_id", "created_on", "updated_on", "started_on", "finished_on"] + list_filter = ["status", "created_on", "started_on", "finished_on"] search_fields = ["id", "owner__username"] ordering = ["-created_on"] diff --git a/batch/migrations/0007_auto_20200122_0827.py b/batch/migrations/0007_auto_20200122_0827.py new file mode 100644 index 0000000..c771045 --- /dev/null +++ b/batch/migrations/0007_auto_20200122_0827.py @@ -0,0 +1,23 @@ +# Generated by Django 2.2.9 on 2020-01-22 08:27 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('batch', '0006_auto_20200117_1459'), + ] + + operations = [ + migrations.AddField( + model_name='job', + name='finished_on', + field=models.DateTimeField(blank=True, editable=False, null=True), + ), + migrations.AddField( + model_name='job', + name='started_on', + field=models.DateTimeField(blank=True, editable=False, null=True), + ), + ] diff --git a/batch/models.py b/batch/models.py index 32da6a7..fa0bc32 100644 --- a/batch/models.py +++ b/batch/models.py @@ -1,5 +1,6 @@ -import uuid +import datetime import enum +import uuid from django.contrib.auth import get_user_model from django.core.exceptions import ValidationError @@ -81,6 +82,14 @@ class JobStatus(enum.Enum): def choices(cls): return [(key.value, key.name) for key in cls] + @property + def is_started(self): + return self == self.running + + @property + def is_finished(self): + return self == self.done or self == self.failed + class Job(models.Model): """ @@ -93,6 +102,8 @@ class Job(models.Model): owner = models.ForeignKey(User, on_delete=models.PROTECT, null=True, blank=True, editable=False) created_on = models.DateTimeField(auto_now_add=True, editable=False) updated_on = models.DateTimeField(auto_now=True, editable=False) + started_on = models.DateTimeField(editable=False, null=True, blank=True) + finished_on = models.DateTimeField(editable=False, null=True, blank=True) # TODO: Should be many to many relation with roles raw_data = models.ForeignKey( @@ -102,3 +113,12 @@ class Job(models.Model): class Meta: verbose_name = "Job" + + def save(self, *args, **kwargs): + now = datetime.datetime.utcnow() + status = JobStatus(self.status) + if status.is_started: + self.started_on = now + elif status.is_finished: + self.finished_on = now + super().save(*args, **kwargs) diff --git a/batch/serializers.py b/batch/serializers.py index 8b319c8..a4c2423 100644 --- a/batch/serializers.py +++ b/batch/serializers.py @@ -43,7 +43,7 @@ def get_viewer_url(self, obj: models.Job) -> Optional[str]: class Meta: model = models.Job - fields = ["id", "status", "created_on", "viewer_url"] + fields = ["id", "status", "created_on", "started_on", "finished_on", "viewer_url"] class BatchJob(serializers.Serializer): diff --git a/cloud_ilastik/templates/batch/project_detail.html b/cloud_ilastik/templates/batch/project_detail.html index 88f0603..b28a4f5 100644 --- a/cloud_ilastik/templates/batch/project_detail.html +++ b/cloud_ilastik/templates/batch/project_detail.html @@ -5,10 +5,10 @@ - + @@ -26,8 +26,9 @@ collected: "table-success", }; - const JobItem = ({index, id, status, created_on, viewer_url}) => { - const time = moment(created_on); + const JobItem = ({id, status, created_on, started_on, finished_on, viewer_url}) => { + const submissionTime = moment(created_on); + let resultElement = null; if (viewer_url) { resultElement = h('a', { @@ -38,11 +39,21 @@ rel: 'noopener', }, 'Open in New Tab'); } + + let runningTimeElement = h('td', null); + if (started_on && finished_on) { + const startedTime = moment(started_on); + const finishedTime = moment(finished_on); + runningTimeElement = h('td', { + title: `Started: ${startedTime}\nFinished: ${finishedTime}`, + }, finishedTime.from(startedTime, true)); + } + return h('tr', {key: id, class: statusClasses[status] || ''}, - h('td', {scope: 'row'}, index + 1), - h('td', null, id.slice(0, 6)), + h('td', {title: id}, id.slice(0, 6)), h('td', null, status), - h('td', {title: time.calendar()}, time.fromNow()), + h('td', {title: submissionTime}, submissionTime.fromNow()), + runningTimeElement, h('td', null, resultElement), ); }; @@ -68,7 +79,7 @@ } render() { - const children = this.state.jobs.map((job, index) => JobItem({...job, index})); + const children = this.state.jobs.map(JobItem); return h(Fragment, null, ...children); } } From 47650e877765cb28be656d5fe8c7c617e96d04f6 Mon Sep 17 00:00:00 2001 From: Emil Melnikov Date: Fri, 24 Jan 2020 11:33:54 +0100 Subject: [PATCH 2/4] Move job status updates to JobManager --- batch/models.py | 43 ++++++++++++++++++++++++++++--------------- batch/views.py | 14 +++++++------- 2 files changed, 35 insertions(+), 22 deletions(-) diff --git a/batch/models.py b/batch/models.py index fa0bc32..95e5904 100644 --- a/batch/models.py +++ b/batch/models.py @@ -82,13 +82,33 @@ class JobStatus(enum.Enum): def choices(cls): return [(key.value, key.name) for key in cls] - @property - def is_started(self): - return self == self.running - @property - def is_finished(self): - return self == self.done or self == self.failed +class JobManager(models.Manager): + _STATUS_TRANSITIONS = { + (JobStatus.created, JobStatus.running): {"now": "started_on"}, + (JobStatus.running, JobStatus.done): {"now": "finished_on"}, + (JobStatus.running, JobStatus.failed): {"now": "finished_on"}, + (JobStatus.done, JobStatus.collected): {}, + } + + def update_status(self, *, old: JobStatus, new: JobStatus) -> int: + """Update the job status of records with the old status to the new status. + + Returns: + Number of changed rows. + + Raises: + ValueError: Status transition is invalid. + """ + transition_info = self._STATUS_TRANSITIONS.get((old, new)) + if transition_info is None: + raise ValueError(f"invalid job status transition from {old.value} to {new.value}") + + extra_update_fields = {} + if "now" in transition_info: + extra_update_fields[transition_info["now"]] = datetime.datetime.utcnow() + + return self.filter(status=old).update(status=new, **extra_update_fields) class Job(models.Model): @@ -111,14 +131,7 @@ class Job(models.Model): ) project = models.ForeignKey(Project, editable=False, null=True, blank=False, on_delete=models.PROTECT) + objects = JobManager() + class Meta: verbose_name = "Job" - - def save(self, *args, **kwargs): - now = datetime.datetime.utcnow() - status = JobStatus(self.status) - if status.is_started: - self.started_on = now - elif status.is_finished: - self.finished_on = now - super().save(*args, **kwargs) diff --git a/batch/views.py b/batch/views.py index f19fca9..896097f 100644 --- a/batch/views.py +++ b/batch/views.py @@ -127,19 +127,19 @@ def update(self, request, external_id: str): serializer = serializers.JobUpdate(data=request.data) serializer.is_valid(raise_exception=True) - status = serializer.data["status"] - - job = get_object_or_404(models.Job, external_id=external_id) - job.status = status - job.save() + try: + status = models.JobStatus(serializer.data["status"]) + models.Job.objects.filter(external_id=external_id).update_status(old=models.JobStatus.running, new=status) + except ValueError: + return response.Response(status=400) - if status == "done": + if status == models.JobStatus.done: dataset_params = { "name": serializer.data["name"], "url": serializer.data["result_url"], "dtype": serializer.data["dtype"], **{k: v for k, v in serializer.data.items() if k.startswith("size_")}, - "job": job, + "job": models.Job.objects.get(external_id=external_id), } datasets_models.Dataset(**dataset_params).save() From 6e0d2cce72feace95cbef0a9fd19aa5e83adccfa Mon Sep 17 00:00:00 2001 From: Emil Melnikov Date: Tue, 28 Jan 2020 11:42:26 +0100 Subject: [PATCH 3/4] Return HTTP 400 if job status hasn't been updated --- batch/views.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/batch/views.py b/batch/views.py index 896097f..472d9f1 100644 --- a/batch/views.py +++ b/batch/views.py @@ -127,9 +127,13 @@ def update(self, request, external_id: str): serializer = serializers.JobUpdate(data=request.data) serializer.is_valid(raise_exception=True) + job_fields = {"external_id": external_id} + try: status = models.JobStatus(serializer.data["status"]) - models.Job.objects.filter(external_id=external_id).update_status(old=models.JobStatus.running, new=status) + n_affected = models.Job.objects.filter(**job_fields).update_status(old=models.JobStatus.running, new=status) + if n_affected != 1: + raise ValueError except ValueError: return response.Response(status=400) @@ -139,7 +143,7 @@ def update(self, request, external_id: str): "url": serializer.data["result_url"], "dtype": serializer.data["dtype"], **{k: v for k, v in serializer.data.items() if k.startswith("size_")}, - "job": models.Job.objects.get(external_id=external_id), + "job": models.Job.objects.get(**job_fields), } datasets_models.Dataset(**dataset_params).save() From c6d9ee007bd0f028818c558ba07b04dc420f5891 Mon Sep 17 00:00:00 2001 From: Emil Melnikov Date: Wed, 29 Jan 2020 14:13:39 +0100 Subject: [PATCH 4/4] Allow multiple jobs submission Closes https://github.com/ilastik/cloud_ilastik/issues/55. --- batch/views.py | 12 +++++++++--- cloud_ilastik/templates/batch/project_new_job.html | 5 ++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/batch/views.py b/batch/views.py index 472d9f1..ea43526 100644 --- a/batch/views.py +++ b/batch/views.py @@ -53,11 +53,17 @@ def post(self, request, *args, **kwargs): project = self.get_object() try: - dataset = self._compatible_datasets.get(pk=int(self.request.POST["dataset"])) - except (KeyError, ValueError, datasets_models.Dataset.DoesNotExist): + prefix = "dataset-" + # Is it an error if there are no datasets, or some datasets in the request are not in the compatible set? + datasets = self._compatible_datasets.filter( + id__in=(int(k[len(prefix):]) for k in self.request.POST if k.startswith(prefix)) + ) + except (KeyError, ValueError): return HttpResponse(status=400) - models.Job.objects.create(project=project, owner=self.request.user, raw_data=dataset) + models.Job.objects.bulk_create( + models.Job(project=project, owner=self.request.user, raw_data=dataset) for dataset in datasets + ) return HttpResponseRedirect(urls.reverse("batch:project-detail", args=[project.pk])) @property diff --git a/cloud_ilastik/templates/batch/project_new_job.html b/cloud_ilastik/templates/batch/project_new_job.html index 758c3af..76b7b46 100644 --- a/cloud_ilastik/templates/batch/project_new_job.html +++ b/cloud_ilastik/templates/batch/project_new_job.html @@ -7,10 +7,9 @@ Select datasets for batch job submission {% for dataset in dataset_list %}
-
# ID Status Submission TimeRunning Time Result