Job Log Model #1030

smk4664 · 2021-10-25T05:06:20Z

Fixes #992, fixes #864, fixes #650, fixes #453

This adds a new model called JobLogEntry to store log messages from jobs. For #650 it removes the conditional rendering, but does not change the JS. I might be able to fix that before the review is complete though.

Here are some screenshots of the changes:

This adds the JobLogEntry and migrations to convert existing logs from JobResult to the new Model.

…tomic

This also removed duplicate views. Ensured that existing functionality was kept between the views.

Since the deletion of git repositories happens outside of the worker, the job_logs db was not able to find the job_resutl. Since this does not run in transaction.atomic, we don't need the job_logs db.

glennmatthews

Very nice work! Thanks for taking this on.

nautobot/docs/additional-features/jobs.md

nautobot/docs/models/extras/jobresult.md

nautobot/extras/datasources/git.py

nautobot/extras/datasources/registry.py

nautobot/extras/tables.py

nautobot/extras/templates/extras/inc/jobresult.html

nautobot/extras/tests/test_jobs.py

nautobot/extras/templates/extras/jobresult.html

Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>

nautobot/docs/models/extras/jobresult.md

nautobot/extras/management/commands/runjob.py

nautobot/extras/models/models.py

nautobot/extras/signals.py

nautobot/extras/tables.py

nautobot/extras/tests/test_jobs.py

glennmatthews · 2021-11-18T18:47:11Z

Can you please do some testing with this changeset and the nautobot-ssot plugin just to verify that the plugin continues to operate as expected?

smk4664 · 2021-11-18T19:00:37Z

Of course!

# Conflicts: # nautobot/extras/tables.py # nautobot/extras/tests/test_models.py

nautobot/docs/models/extras/jobresult.md

nautobot/extras/models/models.py

nautobot/extras/datasources/registry.py

nautobot/extras/models/models.py

nautobot/extras/tables.py

nautobot/extras/tests/test_jobs.py

Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>

This will only happen when including the extras/inc/jobresult.html from within a plugin and not providing the table. This will help plugins such as the Nautobot SSOT with the transition.

glennmatthews

This is looking great!

nautobot/extras/management/commands/runjob.py

nautobot/extras/templates/extras/gitrepository_result.html

nautobot/extras/templates/extras/jobresult.html

nautobot/extras/tests/test_models.py

Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>

glennmatthews

Almost there! This is a complicated set of page templates, appreciate you putting in the effort here! I think there's room for some refinement in how the extras/jobresult.html and extras/inc/jobresult.html pages intersect, happy to spend some time pairing on this tomorrow if you'd like.

nautobot/extras/templates/extras/inc/jobresult.html

smk4664 · 2021-12-06T22:54:52Z

Almost there! This is a complicated set of page templates, appreciate you putting in the effort here! I think there's room for some refinement in how the extras/jobresult.html and extras/inc/jobresult.html pages intersect, happy to spend some time pairing on this tomorrow if you'd like.

I would appreciate the time. It is a big change.

glennmatthews

Looks good to me! Thanks for seeing this through.

jathanism

Sorry to be a thorn in the 11th hour, but there are some concerns about query performance I would like to be discussed before we merge this.

jathanism · 2021-12-13T17:31:21Z

nautobot/core/cli.py

@@ -132,6 +132,10 @@ def _configure_settings(config):
    if settings.METRICS_ENABLED and "postgres" in settings.DATABASES["default"]["ENGINE"]:
        settings.DATABASES["default"]["ENGINE"] = "django_prometheus.db.backends.postgresql"

+    # Create fake db for job logs. This uses the default db, but allows us to save logs within
+    # transaction.atomic().
+    settings.DATABASES["job_logs"] = settings.DATABASES["default"]


This worries me in that it is a magic setting that we're trying to move away from.

Well, if we move away from running jobs in Atomic Transactions then this can go away as well. This is not meant to be modified, so that is why we stuck it here.

nautobot/docs/models/extras/joblogentry.md

nautobot/docs/models/extras/jobresult.md

nautobot/extras/constants.py

jathanism · 2021-12-13T17:37:51Z

nautobot/extras/management/commands/runjob.py

+        groups = set(JobLogEntry.objects.filter(job_result=job_result).values_list("grouping", flat=True))
+        for group in sorted(groups):
+            logs = JobLogEntry.objects.filter(job_result__pk=job_result.pk, grouping=group)


This smells like possible bad query performance to me. Have you considered the following?

Ordering the query by grouping to eliminate the need to use sorted(groups)?

Using prefetch_related on job_result to get all of these objects in a single query, instead of round trips to the database for each group?

I was asked to add sorting to as a set does not have an order. I could change the filter, but it would still be in an unordered set. @glennmatthews was concerned about the sorting.

I will look at the prefetch_related.

Also to note, that this is only for running the jobs from the CLI, which I would think is rare.

jathanism · 2021-12-13T17:38:39Z

nautobot/extras/management/commands/runjob.py

                )
            )

-            for log_entry in attrs["log"]:
-                status = log_entry[1]
+            for log_entry in logs:


Consider using logs.iterator() here since these are single-use instances that you're not mutating.

See: https://docs.djangoproject.com/en/3.2/ref/models/querysets/#iterator

I will look into iterator, but this is only used when running jobs through management commands, which has limited use as they can't take arguments.

Co-authored-by: Jathan McCollum <jathan@gmail.com>

jathanism

glennmatthews · 2021-12-13T21:50:47Z

Bother, this should have been retargeted into develop for 1.2.0 rather than next for 1.3.0; I missed that detail. Looking into sorting that out...

* Create JobLogEntry and migrations. This adds the JobLogEntry and migrations to convert existing logs from JobResult to the new Model. * Remove functions and documentation that is no longer relevent. * Fix tests and allow JobLogEntries to be created outside transaction.atomic * Convert views to use generic.objectview and django-tables2. This also removed duplicate views. Ensured that existing functionality was kept between the views. * Correct git repository deletion bug. Since the deletion of git repositories happens outside of the worker, the job_logs db was not able to find the job_resutl. Since this does not run in transaction.atomic, we don't need the job_logs db. * Update nautobot/docs/additional-features/jobs.md Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com> * Update nautobot/docs/models/extras/jobresult.md Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com> * Update nautobot/docs/additional-features/jobs.md Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com> * Update nautobot/extras/datasources/git.py Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com> * Address page span issue with Additonal Data. * Add truncating of object strings to the JobLogEntry. * Address the rest of the feedback. * Remove unused imports. * Updating Migration order after upstream changes. * Address feedback from PR. * Black recent changes. * Update nautobot/docs/models/extras/jobresult.md Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com> * Update nautobot/extras/models/models.py Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com> * Update nautobot/extras/models/models.py Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com> * Make additional changes recommended by peer review. * Display error message if the job log table is not available. This will only happen when including the extras/inc/jobresult.html from within a plugin and not providing the table. This will help plugins such as the Nautobot SSOT with the transition. * Update nautobot/extras/templates/extras/jobresult.html Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com> * Update nautobot/extras/tests/test_models.py Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com> * Address hanging div's and add back active_tab. * Update nautobot/docs/models/extras/joblogentry.md Co-authored-by: Jathan McCollum <jathan@gmail.com> * Update nautobot/docs/models/extras/jobresult.md Co-authored-by: Jathan McCollum <jathan@gmail.com> * Update nautobot/docs/models/extras/joblogentry.md Co-authored-by: Jathan McCollum <jathan@gmail.com> * Prepend constants with JOB_LOG_ * Black Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com> Co-authored-by: Jathan McCollum <jathan@gmail.com>

smk4664 added 5 commits October 24, 2021 23:31

Create JobLogEntry and migrations.

2d752eb

This adds the JobLogEntry and migrations to convert existing logs from JobResult to the new Model.

Remove functions and documentation that is no longer relevent.

b3aa0b0

Fix tests and allow JobLogEntries to be created outside transaction.a…

99676bd

…tomic

Convert views to use generic.objectview and django-tables2.

0deff07

This also removed duplicate views. Ensured that existing functionality was kept between the views.

Correct git repository deletion bug.

45dbaf4

Since the deletion of git repositories happens outside of the worker, the job_logs db was not able to find the job_resutl. Since this does not run in transaction.atomic, we don't need the job_logs db.

smk4664 mentioned this pull request Oct 25, 2021

Fix issue with inconsistent view regarding the job_jobresult.html page #865

Closed

glennmatthews reviewed Oct 25, 2021

View reviewed changes

smk4664 and others added 8 commits October 25, 2021 12:22

Update nautobot/docs/additional-features/jobs.md

31a72b4

Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>

Update nautobot/docs/models/extras/jobresult.md

121a62f

Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>

Update nautobot/docs/additional-features/jobs.md

31b9f94

Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>

Update nautobot/extras/datasources/git.py

0bc4bbb

Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>

Address page span issue with Additonal Data.

7f7720e

Add truncating of object strings to the JobLogEntry.

d48404a

Address the rest of the feedback.

7928e4a

Remove unused imports.

7c72484

smk4664 requested a review from glennmatthews November 2, 2021 19:55

smk4664 mentioned this pull request Nov 3, 2021

Save Job Data with JobResults to keep track of what triggered the job #1054

Closed

glennmatthews reviewed Nov 18, 2021

View reviewed changes

glennmatthews added this to the v1.2.0 milestone Nov 18, 2021

smk4664 added 4 commits November 20, 2021 10:12

Merge branch 'next' of github.com:nautobot/nautobot into JobResults

6fec78c

# Conflicts: # nautobot/extras/tables.py # nautobot/extras/tests/test_models.py

Updating Migration order after upstream changes.

ad5788c

Address feedback from PR.

8e09e96

Black recent changes.

ed9e2aa

glennmatthews reviewed Nov 22, 2021

View reviewed changes

smk4664 and others added 4 commits November 22, 2021 14:26

Update nautobot/docs/models/extras/jobresult.md

a4d9615

Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>

Update nautobot/extras/models/models.py

ed47263

Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>

Update nautobot/extras/models/models.py

d7fc021

Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>

Make additional changes recommended by peer review.

7e5083e

Display error message if the job log table is not available.

332ba95

This will only happen when including the extras/inc/jobresult.html from within a plugin and not providing the table. This will help plugins such as the Nautobot SSOT with the transition.

smk4664 requested a review from glennmatthews November 29, 2021 20:27

jedelman8 added the status: accepted label Nov 30, 2021

glennmatthews reviewed Dec 6, 2021

View reviewed changes

smk4664 and others added 3 commits December 6, 2021 15:35

Update nautobot/extras/templates/extras/jobresult.html

d0f0585

Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>

Update nautobot/extras/tests/test_models.py

9ed41ca

Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>

Address hanging div's and add back active_tab.

0c0e032

glennmatthews reviewed Dec 6, 2021

View reviewed changes

nautobot/extras/templates/extras/inc/jobresult.html Show resolved Hide resolved

glennmatthews approved these changes Dec 8, 2021

View reviewed changes

glennmatthews mentioned this pull request Dec 13, 2021

JobResult view should robustly handle cases where data format is not as expected #453

Closed

jathanism suggested changes Dec 13, 2021

View reviewed changes

smk4664 and others added 5 commits December 13, 2021 12:57

Update nautobot/docs/models/extras/joblogentry.md

eef30c0

Co-authored-by: Jathan McCollum <jathan@gmail.com>

Update nautobot/docs/models/extras/jobresult.md

721953e

Co-authored-by: Jathan McCollum <jathan@gmail.com>

Update nautobot/docs/models/extras/joblogentry.md

4563c4a

Co-authored-by: Jathan McCollum <jathan@gmail.com>

Prepend constants with JOB_LOG_

4819cfe

Black

6561bbb

jathanism approved these changes Dec 13, 2021

View reviewed changes

glennmatthews merged commit be95323 into nautobot:next Dec 13, 2021

glennmatthews mentioned this pull request Dec 13, 2021

Job Results UI should render log messages immediately #650

Closed

glennmatthews mentioned this pull request Dec 13, 2021

Add REST API and GraphQL for JobLogEntry #1152

Closed

This was referenced Dec 13, 2021

JobResult Page Slow Loading #992

Closed

Convert JobResult logs to separate database model #1003

Closed

Inconsistent job result webpage #864

Closed

glennmatthews mentioned this pull request Jan 6, 2022

Job list view is missing "last results" from latest job run #1237

Closed

glennmatthews mentioned this pull request Feb 10, 2022

Move Job-related models to a separate file; fix JobLogEntry-related tests #1367

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Job Log Model #1030

Job Log Model #1030

smk4664 commented Oct 25, 2021 •

edited

glennmatthews left a comment

glennmatthews commented Nov 18, 2021

smk4664 commented Nov 18, 2021

glennmatthews left a comment

glennmatthews left a comment

smk4664 commented Dec 6, 2021

glennmatthews left a comment

jathanism left a comment

jathanism Dec 13, 2021

smk4664 Dec 13, 2021

jathanism Dec 13, 2021

smk4664 Dec 13, 2021

smk4664 Dec 13, 2021

jathanism Dec 13, 2021

jathanism Dec 13, 2021

smk4664 Dec 13, 2021

jathanism left a comment

glennmatthews commented Dec 13, 2021

Job Log Model #1030

Job Log Model #1030

Conversation

smk4664 commented Oct 25, 2021 • edited

Fixes #992, fixes #864, fixes #650, fixes #453

glennmatthews left a comment

Choose a reason for hiding this comment

glennmatthews commented Nov 18, 2021

smk4664 commented Nov 18, 2021

glennmatthews left a comment

Choose a reason for hiding this comment

glennmatthews left a comment

Choose a reason for hiding this comment

smk4664 commented Dec 6, 2021

glennmatthews left a comment

Choose a reason for hiding this comment

jathanism left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jathanism left a comment

Choose a reason for hiding this comment

glennmatthews commented Dec 13, 2021

smk4664 commented Oct 25, 2021 •

edited