PR review updates

This commit is contained in:
John Anderson
2020-07-03 11:55:04 -04:00
parent 043374946c
commit 38ef8fb4c4
17 changed files with 178 additions and 123 deletions

View File

@@ -258,11 +258,7 @@ class JobResultSerializer(serializers.ModelSerializer):
#
class ReportSerializer(serializers.Serializer):
url = serializers.HyperlinkedIdentityField(
view_name='extras-api:report-detail',
lookup_field='full_name',
lookup_url_kwarg='pk'
)
id = serializers.CharField(read_only=True, source="full_name")
module = serializers.CharField(max_length=255)
name = serializers.CharField(max_length=255)
description = serializers.CharField(max_length=255, required=False)
@@ -279,12 +275,8 @@ class ReportDetailSerializer(ReportSerializer):
#
class ScriptSerializer(serializers.Serializer):
url = serializers.HyperlinkedIdentityField(
view_name='extras-api:script-detail',
lookup_field='full_name',
lookup_url_kwarg='pk'
)
id = serializers.CharField(read_only=True, source="full_name")
module = serializers.CharField(max_length=255)
name = serializers.CharField(read_only=True)
description = serializers.CharField(read_only=True)
vars = serializers.SerializerMethodField(read_only=True)

View File

@@ -14,7 +14,7 @@ from extras.choices import JobResultStatusChoices
from extras.models import (
ConfigContext, CustomFieldChoice, ExportTemplate, Graph, ImageAttachment, ObjectChange, JobResult, Tag,
)
from extras.reports import get_report, get_reports
from extras.reports import get_report, get_reports, run_report
from extras.scripts import get_script, get_scripts, run_script
from utilities.api import IsAuthenticatedOrLoginNotRequired, ModelViewSet
from utilities.utils import copy_safe_request

View File

@@ -129,25 +129,23 @@ class JobResultStatusChoices(ChoiceSet):
STATUS_PENDING = 'pending'
STATUS_RUNNING = 'running'
STATUS_COMPLETED = 'completed'
STATUS_ERRORED = 'errored'
STATUS_FAILED = 'failed'
CHOICES = (
(STATUS_PENDING, 'Pending'),
(STATUS_RUNNING, 'Running'),
(STATUS_COMPLETED, 'Completed'),
(STATUS_ERRORED, 'Errored'),
(STATUS_FAILED, 'Failed'),
)
TERMINAL_STATE_CHOICES = (
STATUS_COMPLETED,
STATUS_ERRORED,
STATUS_FAILED,
)
NON_TERMINAL_STATE_CHOICES = (
STATUS_PENDING,
STATUS_RUNNING,
)
#
# Webhooks

View File

@@ -0,0 +1,22 @@
# Generated by Django 3.0.7 on 2020-06-23 02:28
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('extras', '0042_customfield_manager'),
]
operations = [
migrations.CreateModel(
name='Report',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False)),
],
options={
'managed': False,
},
)
]

View File

@@ -13,15 +13,11 @@ def convert_job_results(apps, schema_editor):
"""
Convert ReportResult objects to JobResult objects
"""
from django.contrib.contenttypes.management import create_contenttypes
from extras.choices import JobResultStatusChoices
ReportResult = apps.get_model('extras', 'ReportResult')
JobResult = apps.get_model('extras', 'JobResult')
ContentType = apps.get_model('contenttypes', 'ContentType')
app_config = apps.get_app_config('extras')
app_config.models_module = app_config.models_module or True
create_contenttypes(app_config)
report_content_type = ContentType.objects.get(app_label='extras', model='report')
job_results = []
@@ -50,19 +46,10 @@ class Migration(migrations.Migration):
dependencies = [
('contenttypes', '0002_remove_content_type_name'),
migrations.swappable_dependency(settings.AUTH_USER_MODEL),
('extras', '0042_customfield_manager'),
('extras', '0043_report_model'),
]
operations = [
migrations.CreateModel(
name='Report',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False)),
],
options={
'managed': False,
},
),
migrations.CreateModel(
name='JobResult',
fields=[

View File

@@ -12,6 +12,7 @@ from django.db import models
from django.http import HttpResponse
from django.template import Template, Context
from django.urls import reverse
from django.utils import timezone
from rest_framework.utils.encoders import JSONEncoder
from utilities.querysets import RestrictedQuerySet
@@ -650,6 +651,18 @@ class JobResult(models.Model):
return f"{int(minutes)} minutes, {seconds:.2f} seconds"
def set_status(self, status):
"""
Helper method to change the status of the job result and save. If the target status is terminal, the
completion time is also set.
"""
self.status = status
if status in JobResultStatusChoices.TERMINAL_STATE_CHOICES:
self.completed = timezone.now()
self.save()
@classmethod
def enqueue_job(cls, func, name, obj_type, user, *args, **kwargs):
"""

View File

@@ -14,6 +14,9 @@ from .constants import *
from .models import JobResult
logger = logging.getLogger(__name__)
def is_report(obj):
"""
Returns True if the given object is a Report.
@@ -71,13 +74,18 @@ def run_report(job_result, *args, **kwargs):
"""
module_name, report_name = job_result.name.split('.', 1)
report = get_report(module_name, report_name)
report.run(job_result)
try:
report.run(job_result)
except Exception:
job_result.set_status(JobResultStatusChoices.STATUS_ERRORED)
logging.error(f"Error during execution of report {job_result.name}")
# Delete any previous terminal state results
JobResult.objects.filter(
obj_type=job_result.obj_type,
name=job_result.name,
status=JobResultStatusChoices.TERMINAL_STATE_CHOICES
status__in=JobResultStatusChoices.TERMINAL_STATE_CHOICES
).exclude(
pk=job_result.pk
).delete()

View File

@@ -399,10 +399,6 @@ def run_script(data, request, commit=True, *args, **kwargs):
A wrapper for calling Script.run(). This performs error handling and provides a hook for committing changes. It
exists outside of the Script class to ensure it cannot be overridden by a script author.
"""
output = None
start_time = None
end_time = None
job_result = kwargs.pop('job_result')
module, script_name = job_result.name.split('.', 1)
@@ -463,7 +459,7 @@ def run_script(data, request, commit=True, *args, **kwargs):
JobResult.objects.filter(
obj_type=job_result.obj_type,
name=job_result.name,
status=JobResultStatusChoices.TERMINAL_STATE_CHOICES
status__in=JobResultStatusChoices.TERMINAL_STATE_CHOICES
).exclude(
pk=job_result.pk
).delete()

View File

@@ -1,8 +1,8 @@
import django_tables2 as tables
from django_tables2.utils import Accessor
from utilities.tables import BaseTable, BooleanColumn, ButtonsColumn, ColorColumn, ToggleColumn
from .models import ConfigContext, ObjectChange, JobResult, Tag, TaggedItem
from utilities.tables import BaseTable, BooleanColumn, ColorColumn, ToggleColumn
from .models import ConfigContext, ObjectChange, Tag, TaggedItem
TAGGED_ITEM = """
{% if value.get_absolute_url %}

View File

@@ -8,9 +8,9 @@ from rest_framework import status
from dcim.models import Device, DeviceRole, DeviceType, Manufacturer, Rack, RackGroup, RackRole, Site
from extras.api.views import ScriptViewSet
from extras.models import ConfigContext, Graph, ExportTemplate, Tag
from extras.reports import Report
from extras.scripts import BooleanVar, IntegerVar, Script, StringVar
from utilities.testing import APITestCase, APIViewTestCases
from utilities.utils import copy_safe_request
class AppTest(APITestCase):
@@ -208,6 +208,37 @@ class ConfigContextTest(APIViewTestCases.APIViewTestCase):
self.assertEqual(rendered_context['bar'], 456)
class ReportTest(APITestCase):
class TestReport(Report):
pass # The report is not actually run, we are testing that the view enqueues the job
def get_test_report(self, *args):
return self.TestReport
def setUp(self):
super().setUp()
# Monkey-patch the API viewset's _get_script method to return our test script above
ReportViewSet._get_report = self.get_test_report
def test_get_report(self):
url = reverse('extras-api:report-detail', kwargs={'pk': None})
response = self.client.get(url, **self.header)
self.assertEqual(response.data['name'], self.TestReport.__name__)
def test_run_report(self):
url = reverse('extras-api:report-detail', kwargs={'pk': None})
response = self.client.post(url, {}, format='json', **self.header)
self.assertHttpStatus(response, status.HTTP_200_OK)
self.assertEqual(response.data['result']['status']['value'], 'pending')
class ScriptTest(APITestCase):
class TestScript(Script):

View File

@@ -36,12 +36,12 @@ urlpatterns = [
# Reports
path('reports/', views.ReportListView.as_view(), name='report_list'),
path('reports/<str:name>/', views.ReportView.as_view(), name='report'),
path('reports/<str:name>/run/', views.ReportRunView.as_view(), name='report_run'),
path('reports/<str:module>.<str:name>/', views.ReportView.as_view(), name='report'),
path('reports/results/<int:job_result_pk>/', views.ReportResultView.as_view(), name='report_result'),
# Scripts
path('scripts/', views.ScriptListView.as_view(), name='script_list'),
path('scripts/<str:module>/<str:name>/', views.ScriptView.as_view(), name='script'),
path('scripts/<str:module>/<str:name>/result/<int:job_result_pk>/', views.ScriptResultView.as_view(), name='script_result'),
path('scripts/<str:module>.<str:name>/', views.ScriptView.as_view(), name='script'),
path('scripts/results/<int:job_result_pk>/', views.ScriptResultView.as_view(), name='script_result'),
]

View File

@@ -350,12 +350,10 @@ class ReportListView(ContentTypePermissionRequiredMixin, View):
class GetReportMixin:
def get_report(self, name):
if '.' not in name:
raise Http404
# Retrieve the Report by "<module>.<report>"
module_name, report_name = name.split('.', 1)
report = get_report(module_name, report_name)
def _get_report(self, name, module=None):
if module is None:
module, name = name.split('.', 1)
report = get_report(module, name)
if report is None:
raise Http404
@@ -369,43 +367,29 @@ class ReportView(GetReportMixin, ContentTypePermissionRequiredMixin, View):
def get_required_permission(self):
return 'extras.view_reportresult'
def get(self, request, name):
def get(self, request, module, name):
report = self.get_report(name)
report = self._get_report(name, module)
report_content_type = ContentType.objects.get(app_label='extras', model='report')
latest_run_result = JobResult.objects.filter(
report.result = JobResult.objects.filter(
obj_type=report_content_type,
name=report.full_name,
status__in=JobResultStatusChoices.TERMINAL_STATE_CHOICES
).first()
pending_run_result = JobResult.objects.filter(
obj_type=report_content_type,
name=report.full_name,
status__in=JobResultStatusChoices.NON_TERMINAL_STATE_CHOICES
).order_by(
'created'
).first()
report.result = latest_run_result
report.pending_result = pending_run_result
return render(request, 'extras/report.html', {
'report': report,
'run_form': ConfirmationForm(),
})
def post(self, request, module, name):
class ReportRunView(GetReportMixin, ContentTypePermissionRequiredMixin, View):
"""
Run a Report and record a new ReportResult.
"""
def get_required_permission(self):
return 'extras.add_reportresult'
# Permissions check
if not request.user.has_perm('extras.run_report'):
return HttpResponseForbidden()
def post(self, request, name):
report = self.get_report(name)
report = self._get_report(name, module)
form = ConfirmationForm(request.POST)
if form.is_valid():
@@ -419,15 +403,42 @@ class ReportRunView(GetReportMixin, ContentTypePermissionRequiredMixin, View):
request.user
)
return redirect('extras:report', name=report.full_name)
return redirect('extras:report_result', job_result_pk=job_result.pk)
return render(request, 'extras/report.html', {
'report': report,
'run_form': form,
})
class ReportResultView(ContentTypePermissionRequiredMixin, GetReportMixin, View):
def get_required_permission(self):
return 'extras.view_report'
def get(self, request, job_result_pk):
result = get_object_or_404(JobResult.objects.all(), pk=job_result_pk)
report_content_type = ContentType.objects.get(app_label='extras', model='report')
if result.obj_type != report_content_type:
raise Http404
report = self._get_report(result.name)
return render(request, 'extras/report_result.html', {
'report': report,
'result': result,
'class_name': report.name,
'run_form': ConfirmationForm(),
})
#
# Scripts
#
class GetScriptMixin:
def _get_script(self, module, name):
def _get_script(self, name, module=None):
if module is None:
module, name = name.split('.', 1)
scripts = get_scripts()
try:
return scripts[module][name]()
@@ -453,7 +464,7 @@ class ScriptView(ContentTypePermissionRequiredMixin, GetScriptMixin, View):
return 'extras.view_script'
def get(self, request, module, name):
script = self._get_script(module, name)
script = self._get_script(name, module)
form = script.as_form(initial=request.GET)
# Look for a pending JobResult (use the latest one by creation timestamp)
@@ -461,7 +472,8 @@ class ScriptView(ContentTypePermissionRequiredMixin, GetScriptMixin, View):
script.result = JobResult.objects.filter(
obj_type=script_content_type,
name=script.full_name,
status__in=JobResultStatusChoices.NON_TERMINAL_STATE_CHOICES
).exclude(
status__in=JobResultStatusChoices.TERMINAL_STATE_CHOICES
).first()
return render(request, 'extras/script.html', {
@@ -476,10 +488,8 @@ class ScriptView(ContentTypePermissionRequiredMixin, GetScriptMixin, View):
if not request.user.has_perm('extras.run_script'):
return HttpResponseForbidden()
script = self._get_script(module, name)
script = self._get_script(name, module)
form = script.as_form(request.POST, request.FILES)
output = None
execution_time = None
if form.is_valid():
commit = form.cleaned_data.pop('_commit')
@@ -495,7 +505,13 @@ class ScriptView(ContentTypePermissionRequiredMixin, GetScriptMixin, View):
commit=commit
)
return redirect('extras:script_result', module=module, name=name, job_result_pk=job_result.pk)
return redirect('extras:script_result', job_result_pk=job_result.pk)
return render(request, 'extras/script.html', {
'module': module,
'script': script,
'form': form,
})
class ScriptResultView(ContentTypePermissionRequiredMixin, GetScriptMixin, View):
@@ -503,19 +519,16 @@ class ScriptResultView(ContentTypePermissionRequiredMixin, GetScriptMixin, View)
def get_required_permission(self):
return 'extras.view_script'
def get(self, request, module, name, job_result_pk):
script = self._get_script(module, name)
form = script.as_form(initial=request.GET)
# Look for a pending JobResult (use the latest one by creation timestamp)
script_content_type = ContentType.objects.get(app_label='extras', model='script')
def get(self, request, job_result_pk):
result = get_object_or_404(JobResult.objects.all(), pk=job_result_pk)
script_content_type = ContentType.objects.get(app_label='extras', model='script')
if result.obj_type != script_content_type:
raise Http404
script = self._get_script(result.name)
return render(request, 'extras/script_result.html', {
'module': module,
'script': script,
'result': result,
'class_name': name
'class_name': script.__class__.__name__
})