#12081: Script & report cleanup (#12091)

* start() and terminate() methods on Job should call save()

* Fix display of associated jobs

* Introduce get_latest_jobs() method on JobsMixin

* Update messaging when no reports/scripts exist

* Catch ImportErrors when rendering report/script lists

* Fix loading of nested modules

* Fix URLs for nested scripts/reports

netbox/core/models/jobs.py

@@ -39,7 +39,8 @@ class Job(models.Model):
     )
     object = GenericForeignKey(
         ct_field='object_type',
-        fk_field='object_id'
+        fk_field='object_id',
+        for_concrete_model=False
     )
     name = models.CharField(
         max_length=200
@@ -140,7 +141,7 @@ class Job(models.Model):
         # Start the job
         self.started = timezone.now()
         self.status = JobStatusChoices.STATUS_RUNNING
-        Job.objects.filter(pk=self.pk).update(started=self.started, status=self.status)
+        self.save()
 
         # Handle webhooks
         self.trigger_webhooks(event=EVENT_JOB_START)
@@ -156,7 +157,7 @@ class Job(models.Model):
         # Mark the job as completed
         self.status = status
         self.completed = timezone.now()
-        Job.objects.filter(pk=self.pk).update(status=self.status, completed=self.completed)
+        self.save()
 
         # Handle webhooks
         self.trigger_webhooks(event=EVENT_JOB_END)

netbox/extras/migrations/0091_create_managedfiles.py

@@ -8,16 +8,9 @@ import extras.models.mixins
 
 def create_files(cls, root_name, root_path):
 
-    path_tree = [
+    modules = list(pkgutil.iter_modules([root_path]))
-        path for path, _, _ in os.walk(root_path)
-        if os.path.basename(path)[0] not in ('_', '.')
-    ]
-
-    modules = list(pkgutil.iter_modules(path_tree))
     filenames = []
-    for importer, module_name, is_pkg in modules:
+    for importer, module_name, ispkg in modules:
-        if is_pkg:
-            continue
         try:
             module = importer.find_module(module_name).load_module(module_name)
             rel_path = os.path.relpath(module.__file__, root_path)

netbox/extras/models/mixins.py

@@ -1,5 +1,5 @@
 import os
-from pkgutil import ModuleInfo, get_importer
+from importlib.machinery import SourceFileLoader
 
 __all__ = (
     'PythonModuleMixin',
@@ -12,16 +12,17 @@ class PythonModuleMixin:
     def path(self):
         return os.path.splitext(self.file_path)[0]
 
-    def get_module_info(self):
+    @property
-        path = os.path.dirname(self.full_path)
+    def python_name(self):
-        module_name = os.path.basename(self.path)
+        path, filename = os.path.split(self.full_path)
-        return ModuleInfo(
+        name = os.path.splitext(filename)[0]
-            module_finder=get_importer(path),
+        if name == '__init__':
-            name=module_name,
+            # File is a package
-            ispkg=False
+            return os.path.basename(path)
-        )
+        else:
+            return name
 
     def get_module(self):
-        importer, module_name, _ = self.get_module_info()
+        loader = SourceFileLoader(self.python_name, self.full_path)
-        module = importer.find_module(module_name).load_module(module_name)
+        module = loader.load_module()
         return module

netbox/extras/models/reports.py

@@ -44,6 +44,9 @@ class ReportModule(PythonModuleMixin, JobsMixin, ManagedFile):
     def get_absolute_url(self):
         return reverse('extras:report_list')
 
+    def __str__(self):
+        return self.python_name
+
     @cached_property
     def reports(self):
 
@@ -51,7 +54,10 @@ class ReportModule(PythonModuleMixin, JobsMixin, ManagedFile):
             # For child objects in submodules use the full import path w/o the root module as the name
             return cls.full_name.split(".", maxsplit=1)[1]
 
-        module = self.get_module()
+        try:
+            module = self.get_module()
+        except ImportError:
+            return {}
         reports = {}
         ordered = getattr(module, 'report_order', [])
 

netbox/extras/models/scripts.py

@@ -1,7 +1,6 @@
 import inspect
 from functools import cached_property
 
-from django.contrib.contenttypes.fields import GenericRelation
 from django.db import models
 from django.urls import reverse
 
@@ -44,6 +43,9 @@ class ScriptModule(PythonModuleMixin, JobsMixin, ManagedFile):
     def get_absolute_url(self):
         return reverse('extras:script_list')
 
+    def __str__(self):
+        return self.python_name
+
     @cached_property
     def scripts(self):
 

netbox/extras/reports.py

@@ -195,8 +195,6 @@ class Report(object):
         Run the report and save its results. Each test method will be executed in order.
         """
         self.logger.info(f"Running report")
-        job.status = JobStatusChoices.STATUS_RUNNING
-        job.save()
 
         # Perform any post-run tasks
         self.pre_run()
@@ -218,6 +216,7 @@ class Report(object):
             logger.error(f"Exception raised during report execution: {e}")
             job.terminate(status=JobStatusChoices.STATUS_ERRORED)
         finally:
+            job.data = self._results
             job.terminate()
 
         # Perform any post-run tasks

netbox/extras/urls.py

@@ -97,17 +97,17 @@ urlpatterns = [
     path('reports/add/', views.ReportModuleCreateView.as_view(), name='reportmodule_add'),
     path('reports/results/<int:job_pk>/', views.ReportResultView.as_view(), name='report_result'),
     path('reports/<int:pk>/', include(get_model_urls('extras', 'reportmodule'))),
-    path('reports/<path:module>.<str:name>/', views.ReportView.as_view(), name='report'),
+    path('reports/<str:module>/<str:name>/', views.ReportView.as_view(), name='report'),
-    path('reports/<path:module>.<str:name>/jobs/', views.ReportJobsView.as_view(), name='report_jobs'),
+    path('reports/<str:module>/<str:name>/jobs/', views.ReportJobsView.as_view(), name='report_jobs'),
 
     # Scripts
     path('scripts/', views.ScriptListView.as_view(), name='script_list'),
     path('scripts/add/', views.ScriptModuleCreateView.as_view(), name='scriptmodule_add'),
     path('scripts/results/<int:job_pk>/', views.ScriptResultView.as_view(), name='script_result'),
     path('scripts/<int:pk>/', include(get_model_urls('extras', 'scriptmodule'))),
-    path('scripts/<path:module>.<str:name>/', views.ScriptView.as_view(), name='script'),
+    path('scripts/<str:module>/<str:name>/', views.ScriptView.as_view(), name='script'),
-    path('scripts/<path:module>.<str:name>/source/', views.ScriptSourceView.as_view(), name='script_source'),
+    path('scripts/<str:module>/<str:name>/source/', views.ScriptSourceView.as_view(), name='script_source'),
-    path('scripts/<path:module>.<str:name>/jobs/', views.ScriptJobsView.as_view(), name='script_jobs'),
+    path('scripts/<str:module>/<str:name>/jobs/', views.ScriptJobsView.as_view(), name='script_jobs'),
 
     # Markdown
     path('render/markdown/', views.RenderMarkdownView.as_view(), name="render_markdown")

netbox/extras/views.py

@@ -819,19 +819,9 @@ class ReportListView(ContentTypePermissionRequiredMixin, View):
     def get(self, request):
         report_modules = ReportModule.objects.restrict(request.user)
 
-        report_content_type = ContentType.objects.get(app_label='extras', model='report')
-        jobs = {
-            r.name: r
-            for r in Job.objects.filter(
-                object_type=report_content_type,
-                status__in=JobStatusChoices.TERMINAL_STATE_CHOICES
-            ).order_by('name', '-created').distinct('name').defer('data')
-        }
-
         return render(request, 'extras/report_list.html', {
             'model': ReportModule,
             'report_modules': report_modules,
-            'jobs': jobs,
         })
 
 
@@ -843,7 +833,7 @@ class ReportView(ContentTypePermissionRequiredMixin, View):
         return 'extras.view_report'
 
     def get(self, request, module, name):
-        module = get_object_or_404(ReportModule.objects.restrict(request.user), file_path=f'{module}.py')
+        module = get_object_or_404(ReportModule.objects.restrict(request.user), file_path__startswith=module)
         report = module.reports[name]()
 
         object_type = ContentType.objects.get(app_label='extras', model='reportmodule')
@@ -864,7 +854,7 @@ class ReportView(ContentTypePermissionRequiredMixin, View):
         if not request.user.has_perm('extras.run_report'):
             return HttpResponseForbidden()
 
-        module = get_object_or_404(ReportModule.objects.restrict(request.user), file_path=f'{module}.py')
+        module = get_object_or_404(ReportModule.objects.restrict(request.user), file_path__startswith=module)
         report = module.reports[name]()
         form = ReportForm(request.POST)
 
@@ -903,7 +893,7 @@ class ReportJobsView(ContentTypePermissionRequiredMixin, View):
         return 'extras.view_report'
 
     def get(self, request, module, name):
-        module = get_object_or_404(ReportModule.objects.restrict(request.user), file_path=f'{module}.py')
+        module = get_object_or_404(ReportModule.objects.restrict(request.user), file_path__startswith=module)
         report = module.reports[name]()
 
         object_type = ContentType.objects.get(app_label='extras', model='reportmodule')
@@ -987,19 +977,9 @@ class ScriptListView(ContentTypePermissionRequiredMixin, View):
     def get(self, request):
         script_modules = ScriptModule.objects.restrict(request.user)
 
-        script_content_type = ContentType.objects.get(app_label='extras', model='script')
-        jobs = {
-            r.name: r
-            for r in Job.objects.filter(
-                object_type=script_content_type,
-                status__in=JobStatusChoices.TERMINAL_STATE_CHOICES
-            ).order_by('name', '-created').distinct('name').defer('data')
-        }
-
         return render(request, 'extras/script_list.html', {
             'model': ScriptModule,
             'script_modules': script_modules,
-            'jobs': jobs,
         })
 
 
@@ -1009,7 +989,8 @@ class ScriptView(ContentTypePermissionRequiredMixin, View):
         return 'extras.view_script'
 
     def get(self, request, module, name):
-        module = get_object_or_404(ScriptModule.objects.restrict(request.user), file_path=f'{module}.py')
+        print(module)
+        module = get_object_or_404(ScriptModule.objects.restrict(request.user), file_path__startswith=module)
         script = module.scripts[name]()
         form = script.as_form(initial=normalize_querydict(request.GET))
 
@@ -1033,7 +1014,7 @@ class ScriptView(ContentTypePermissionRequiredMixin, View):
         if not request.user.has_perm('extras.run_script'):
             return HttpResponseForbidden()
 
-        module = get_object_or_404(ScriptModule.objects.restrict(request.user), file_path=f'{module}.py')
+        module = get_object_or_404(ScriptModule.objects.restrict(request.user), file_path__startswith=module)
         script = module.scripts[name]()
         form = script.as_form(request.POST, request.FILES)
 
@@ -1070,7 +1051,7 @@ class ScriptSourceView(ContentTypePermissionRequiredMixin, View):
         return 'extras.view_script'
 
     def get(self, request, module, name):
-        module = get_object_or_404(ScriptModule.objects.restrict(request.user), file_path=f'{module}.py')
+        module = get_object_or_404(ScriptModule.objects.restrict(request.user), file_path__startswith=module)
         script = module.scripts[name]()
 
         return render(request, 'extras/script/source.html', {
@@ -1086,7 +1067,7 @@ class ScriptJobsView(ContentTypePermissionRequiredMixin, View):
         return 'extras.view_script'
 
     def get(self, request, module, name):
-        module = get_object_or_404(ScriptModule.objects.restrict(request.user), file_path=f'{module}.py')
+        module = get_object_or_404(ScriptModule.objects.restrict(request.user), file_path__startswith=module)
         script = module.scripts[name]()
 
         object_type = ContentType.objects.get(app_label='extras', model='scriptmodule')

netbox/netbox/models/features.py

@@ -9,9 +9,9 @@ from django.db.models.signals import class_prepared
 from django.dispatch import receiver
 from django.utils import timezone
 from django.utils.translation import gettext as _
-
 from taggit.managers import TaggableManager
 
+from core.choices import JobStatusChoices
 from extras.choices import CustomFieldVisibilityChoices, ObjectChangeActionChoices
 from extras.utils import is_taggable, register_features
 from netbox.registry import registry
@@ -302,12 +302,24 @@ class JobsMixin(models.Model):
     jobs = GenericRelation(
         to='core.Job',
         content_type_field='object_type',
-        object_id_field='object_id'
+        object_id_field='object_id',
+        for_concrete_model=False
     )
 
     class Meta:
         abstract = True
 
+    def get_latest_jobs(self):
+        """
+        Return a dictionary mapping of the most recent jobs for this instance.
+        """
+        return {
+            job.name: job
+            for job in self.jobs.filter(
+                status__in=JobStatusChoices.TERMINAL_STATE_CHOICES
+            ).order_by('name', '-created').distinct('name').defer('data')
+        }
+
 
 class JournalingMixin(models.Model):
     """

netbox/templates/extras/report_list.html

@@ -34,7 +34,7 @@
               </a>
             </div>
           {% endif %}
-          <i class="mdi mdi-file-document-outline"></i> {{ module.name|bettertitle }}
+          <i class="mdi mdi-file-document-outline"></i> {{ module }}
         </h5>
         <div class="card-body">
           {% include 'inc/sync_warning.html' with object=module %}
@@ -49,56 +49,58 @@
               </tr>
             </thead>
             <tbody>
-              {% for report_name, report in module.reports.items %}
+              {% with jobs=module.get_latest_jobs %}
-                {% with last_result=jobs|get_key:report.full_name %}
+                {% for report_name, report in module.reports.items %}
-                  <tr>
+                  {% with last_job=jobs|get_key:report.name %}
-                    <td>
-                      <a href="{% url 'extras:report' module=module.path name=report.class_name %}" id="{{ report.module }}.{{ report.class_name }}">{{ report.name }}</a>
-                    </td>
-                    <td>{{ report.description|markdown|placeholder }}</td>
-                    {% if last_result %}
-                      <td>
-                        <a href="{% url 'extras:report_result' job_pk=last_result.pk %}">{{ last_result.created|annotated_date }}</a>
-                      </td>
-                      <td>
-                        {% badge last_result.get_status_display last_result.get_status_color %}
-                      </td>
-                    {% else %}
-                      <td class="text-muted">Never</td>
-                      <td>{{ ''|placeholder }}</td>
-                    {% endif %}
-                    <td>
-                      {% if perms.extras.run_report %}
-                        <div class="float-end noprint">
-                          <form action="{% url 'extras:report' module=report.module name=report.class_name %}" method="post">
-                            {% csrf_token %}
-                            <button type="submit" name="_run" class="btn btn-primary btn-sm" style="width: 110px">
-                              {% if last_result %}
-                                <i class="mdi mdi-replay"></i> Run Again
-                              {% else %}
-                                <i class="mdi mdi-play"></i> Run Report
-                              {% endif %}
-                            </button>
-                          </form>
-                        </div>
-                      {% endif %}
-                    </td>
-                  </tr>
-                  {% for method, stats in last_result.data.items %}
                     <tr>
-                      <td colspan="4" class="method">
+                      <td>
-                        <span class="ps-3">{{ method }}</span>
+                        <a href="{% url 'extras:report' module=module.python_name name=report.class_name %}" id="{{ report.module }}.{{ report.class_name }}">{{ report.name }}</a>
                       </td>
-                      <td class="text-end text-nowrap report-stats">
+                      <td>{{ report.description|markdown|placeholder }}</td>
-                        <span class="badge bg-success">{{ stats.success }}</span>
+                      {% if last_job %}
-                        <span class="badge bg-info">{{ stats.info }}</span>
+                        <td>
-                        <span class="badge bg-warning">{{ stats.warning }}</span>
+                          <a href="{% url 'extras:report_result' job_pk=last_job.pk %}">{{ last_job.created|annotated_date }}</a>
-                        <span class="badge bg-danger">{{ stats.failure }}</span>
+                        </td>
+                        <td>
+                          {% badge last_job.get_status_display last_job.get_status_color %}
+                        </td>
+                      {% else %}
+                        <td class="text-muted">Never</td>
+                        <td>{{ ''|placeholder }}</td>
+                      {% endif %}
+                      <td>
+                        {% if perms.extras.run_report %}
+                          <div class="float-end noprint">
+                            <form action="{% url 'extras:report' module=report.module name=report.class_name %}" method="post">
+                              {% csrf_token %}
+                              <button type="submit" name="_run" class="btn btn-primary btn-sm" style="width: 110px">
+                                {% if last_job %}
+                                  <i class="mdi mdi-replay"></i> Run Again
+                                {% else %}
+                                  <i class="mdi mdi-play"></i> Run Report
+                                {% endif %}
+                              </button>
+                            </form>
+                          </div>
+                        {% endif %}
                       </td>
                     </tr>
-                  {% endfor %}
+                    {% for method, stats in last_job.data.items %}
-                {% endwith %}
+                      <tr>
-              {% endfor %}
+                        <td colspan="4" class="method">
+                          <span class="ps-3">{{ method }}</span>
+                        </td>
+                        <td class="text-end text-nowrap report-stats">
+                          <span class="badge bg-success">{{ stats.success }}</span>
+                          <span class="badge bg-info">{{ stats.info }}</span>
+                          <span class="badge bg-warning">{{ stats.warning }}</span>
+                          <span class="badge bg-danger">{{ stats.failure }}</span>
+                        </td>
+                      </tr>
+                    {% endfor %}
+                  {% endwith %}
+                {% endfor %}
+              {% endwith %}
             </tbody>
           </table>
         </div>
@@ -106,9 +108,9 @@
     {% empty %}
       <div class="alert alert-info" role="alert">
         <h4 class="alert-heading">No Reports Found</h4>
-        Reports should be saved to <code>{{ settings.REPORTS_ROOT }}</code>.
+        {% if perms.extras.add_reportmodule %}
-        <hr/>
+          Get started by <a href="{% url 'extras:reportmodule_add' %}">creating a report</a> from an uploaded file or data source.
-        <small>This path can be changed by setting <code>REPORTS_ROOT</code> in NetBox's configuration.</small>
+        {% endif %}
       </div>
     {% endfor %}
   </div>

netbox/templates/extras/script_list.html

@@ -33,7 +33,7 @@
               </a>
             </div>
           {% endif %}
-          <i class="mdi mdi-file-document-outline"></i> {{ module.name|bettertitle }}
+          <i class="mdi mdi-file-document-outline"></i> {{ module }}
         </h5>
         <div class="card-body">
           {% include 'inc/sync_warning.html' with object=module %}
@@ -47,29 +47,31 @@
               </tr>
             </thead>
             <tbody>
-              {% for script_name, script_class in module.scripts.items %}
+              {% with jobs=module.get_latest_jobs %}
-                {% with last_result=jobs|get_key:script_class.full_name %}
+                {% for script_name, script_class in module.scripts.items %}
                   <tr>
                     <td>
-                      <a href="{% url 'extras:script' module=module.path name=script_name %}" name="script.{{ script_name }}">{{ script_class.name }}</a>
+                      <a href="{% url 'extras:script' module=module.python_name name=script_name %}" name="script.{{ script_name }}">{{ script_class.name }}</a>
                     </td>
                     <td>
                       {{ script_class.Meta.description|markdown|placeholder }}
                     </td>
-                    {% if last_result %}
+                    {% with last_result=jobs|get_key:script_class.name %}
-                      <td>
+                      {% if last_result %}
-                        <a href="{% url 'extras:script_result' job_pk=last_result.pk %}">{{ last_result.created|annotated_date }}</a>
+                        <td>
-                      </td>
+                          <a href="{% url 'extras:script_result' job_pk=last_result.pk %}">{{ last_result.created|annotated_date }}</a>
-                      <td class="text-end">
+                        </td>
-                        {% badge last_result.get_status_display last_result.get_status_color %}
+                        <td class="text-end">
-                      </td>
+                          {% badge last_result.get_status_display last_result.get_status_color %}
-                    {% else %}
+                        </td>
-                      <td class="text-muted">Never</td>
+                      {% else %}
-                      <td class="text-end">{{ ''|placeholder }}</td>
+                        <td class="text-muted">Never</td>
-                    {% endif %}
+                        <td class="text-end">{{ ''|placeholder }}</td>
+                      {% endif %}
+                    {% endwith %}
                   </tr>
-                {% endwith %}
+                {% endfor %}
-              {% endfor %}
+              {% endwith %}
             </tbody>
           </table>
         </div>
@@ -77,9 +79,9 @@
     {% empty %}
       <div class="alert alert-info">
         <h4 class="alert-heading">No Scripts Found</h4>
-        Scripts should be saved to <code>{{ settings.SCRIPTS_ROOT }}</code>.
+        {% if perms.extras.add_scriptmodule %}
-        <hr/>
+          Get started by <a href="{% url 'extras:scriptmodule_add' %}">creating a script</a> from an uploaded file or data source.
-        This path can be changed by setting <code>SCRIPTS_ROOT</code> in NetBox's configuration.
+        {% endif %}
       </div>
     {% endfor %}
   </div>