Repository: ansible/awx
File: awx/main/models/unified_jobs.py (Python)

Code before:
        # Sanity check: Has the job just started? If so, mark down its start
        # time.
        if self.status == 'running' and not self.started:
            self.started = now()
            if 'started' not in update_fields:
                update_fields.append('started')

        # Sanity check: Has the job just completed? If so, mark down its
        # completion time, and record its output to the database.
        if self.status in ('successful', 'failed', 'error', 'canceled') and not self.finished:
            # Record the `finished` time.
            self.finished = now()
            if 'finished' not in update_fields:
                update_fields.append('finished')

        # If we have a start and finished time, and haven't already calculated
        # out the time that elapsed, do so.
        if self.started and self.finished and not self.elapsed:
            td = self.finished - self.started
            elapsed = (td.microseconds + (td.seconds + td.days * 24 * 3600) * 10 ** 6) / (10 ** 6 * 1.0)
        else:
            elapsed = 0.0
        if self.elapsed != elapsed:
            self.elapsed = str(elapsed)
            if 'elapsed' not in update_fields:
                update_fields.append('elapsed')

        # Ensure that the job template information is current.
        if self.unified_job_template != self._get_parent_instance():
            self.unified_job_template = self._get_parent_instance()
            if 'unified_job_template' not in update_fields:
                update_fields.append('unified_job_template')

        if self.cancel_flag and not self.canceled_on:
            # Record the 'canceled' time.
            self.canceled_on = now()
            if 'canceled_on' not in update_fields:
                update_fields.append('canceled_on')
        # Okay; we're done. Perform the actual save.
        result = super(UnifiedJob, self).save(*args, **kwargs)

        # If status changed, update the parent instance.
        if self.status != status_before:
            # Update parent outside of the transaction for Job w/ allow_simultaneous=True
            # This dodges lock contention at the expense of the foreign key not being
            # completely correct.
            if getattr(self, 'allow_simultaneous', False):
                connection.on_commit(self._update_parent_instance)
            else:
                self._update_parent_instance()


Review comment (suggestion):
I would only have 1 minor tweak to suggest - before hitting this line, add another conditional line like

```python
if elapsed != self.elapsed:
```

Otherwise, it looks like every save after a job finished would add this `elapsed` field to the payload for updating the row. It would be preferable and intuitive that it updates the field exactly once, on the first save after the `finished` field is filled in.

Code after:

        # Sanity check: Has the job just started? If so, mark down its start
        # time.
        if self.status == 'running' and not self.started:
            self.started = now()
            if 'started' not in update_fields:
                update_fields.append('started')

        # Sanity check: Has the job just completed? If so, mark down its
        # completion time, and record its output to the database.
        if self.status in ('successful', 'failed', 'error', 'canceled') and not self.finished:
            # Record the `finished` time.
            self.finished = now()
            if 'finished' not in update_fields:
                update_fields.append('finished')

        dq = decimal.Decimal('1.000')
        if self.elapsed is None:
            self.elapsed = decimal.Decimal(0.0).quantize(dq)

        # If we have a start and finished time, and haven't already calculated
        # out the time that elapsed, do so.
        if self.started and self.finished and self.elapsed == 0.0:
            td = self.finished - self.started
            elapsed = decimal.Decimal(td.total_seconds())
            self.elapsed = elapsed.quantize(dq)
            if 'elapsed' not in update_fields:
                update_fields.append('elapsed')

        # Ensure that the job template information is current.
        if self.unified_job_template != self._get_parent_instance():
            self.unified_job_template = self._get_parent_instance()
            if 'unified_job_template' not in update_fields:
                update_fields.append('unified_job_template')

        if self.cancel_flag and not self.canceled_on:
            # Record the 'canceled' time.
            self.canceled_on = now()
            if 'canceled_on' not in update_fields:
                update_fields.append('canceled_on')
        # Okay; we're done. Perform the actual save.
        result = super(UnifiedJob, self).save(*args, **kwargs)

        # If status changed, update the parent instance.
        if self.status != status_before:
            # Update parent outside of the transaction for Job w/ allow_simultaneous=True
            # This dodges lock contention at the expense of the foreign key not being
            # completely correct.
            if getattr(self, 'allow_simultaneous', False):
                connection.on_commit(self._update_parent_instance)
            else: