From 41f92ef8e65ea0f975bb0b80195fb64b59fcd82b Mon Sep 17 00:00:00 2001 From: John Anderson Date: Mon, 6 Jul 2020 01:58:28 -0400 Subject: [PATCH] review updates --- netbox/extras/choices.py | 37 ++++++++++++++++++++++ netbox/extras/constants.py | 14 -------- netbox/extras/reports.py | 22 ++++++------- netbox/extras/scripts.py | 26 +++++++-------- netbox/extras/templatetags/log_levels.py | 29 +++-------------- netbox/extras/views.py | 16 +++++++++- netbox/project-static/js/job_result.js | 6 ++-- netbox/templates/extras/inc/job_label.html | 4 ++- netbox/templates/extras/report_result.html | 6 ++-- netbox/templates/extras/script_list.html | 37 ++++++++++++++++++++-- netbox/templates/extras/script_result.html | 8 ++++- 11 files changed, 130 insertions(+), 75 deletions(-) diff --git a/netbox/extras/choices.py b/netbox/extras/choices.py index 7ff8aab95..fe5c74fa5 100644 --- a/netbox/extras/choices.py +++ b/netbox/extras/choices.py @@ -120,6 +120,43 @@ class TemplateLanguageChoices(ChoiceSet): } +# +# Log Levels for Reports and Scripts +# + +class LogLevelChoices(ChoiceSet): + + LOG_DEFAULT = 'default' + LOG_SUCCESS = 'sucess' + LOG_INFO = 'info' + LOG_WARNING = 'warning' + LOG_FAILURE = 'failure' + + CHOICES = ( + (LOG_DEFAULT, 'Default'), + (LOG_SUCCESS, 'Success'), + (LOG_INFO, 'Info'), + (LOG_WARNING, 'Warning'), + (LOG_FAILURE, 'Failure'), + ) + + CLASS_MAP = ( + (LOG_DEFAULT, 'default'), + (LOG_SUCCESS, 'success'), + (LOG_INFO, 'info'), + (LOG_WARNING, 'warning'), + (LOG_FAILURE, 'danger'), + ) + + LEGACY_MAP = ( + (LOG_DEFAULT, 0), + (LOG_SUCCESS, 10), + (LOG_INFO, 20), + (LOG_WARNING, 30), + (LOG_FAILURE, 40), + ) + + # # Job results # diff --git a/netbox/extras/constants.py b/netbox/extras/constants.py index 55f3c73c7..a506d5867 100644 --- a/netbox/extras/constants.py +++ b/netbox/extras/constants.py @@ -1,17 +1,3 @@ -# Report logging levels -LOG_DEFAULT = 0 -LOG_SUCCESS = 10 -LOG_INFO = 20 -LOG_WARNING = 30 -LOG_FAILURE = 40 -LOG_LEVEL_CODES = { - LOG_DEFAULT: 'default', - LOG_SUCCESS: 'success', - LOG_INFO: 'info', - LOG_WARNING: 'warning', - LOG_FAILURE: 'failure', -} - # Webhook content types HTTP_CONTENT_TYPE_JSON = 'application/json' diff --git a/netbox/extras/reports.py b/netbox/extras/reports.py index db6e8d373..45496d396 100644 --- a/netbox/extras/reports.py +++ b/netbox/extras/reports.py @@ -9,8 +9,7 @@ from django.db.models import Q from django.utils import timezone from django_rq import job -from .choices import JobResultStatusChoices -from .constants import * +from .choices import JobResultStatusChoices, LogLevelChoices from .models import JobResult @@ -77,7 +76,8 @@ def run_report(job_result, *args, **kwargs): try: report.run(job_result) - except Exception: + except Exception as e: + print(e) job_result.set_status(JobResultStatusChoices.STATUS_ERRORED) logging.error(f"Error during execution of report {job_result.name}") @@ -153,15 +153,15 @@ class Report(object): def full_name(self): return '.'.join([self.__module__, self.__class__.__name__]) - def _log(self, obj, message, level=LOG_DEFAULT): + def _log(self, obj, message, level=LogLevelChoices.LOG_DEFAULT): """ Log a message from a test method. Do not call this method directly; use one of the log_* wrappers below. """ - if level not in LOG_LEVEL_CODES: + if level not in LogLevelChoices.as_dict(): raise Exception("Unknown logging level: {}".format(level)) self._results[self.active_test]['log'].append(( timezone.now().isoformat(), - LOG_LEVEL_CODES.get(level), + level, str(obj) if obj else None, obj.get_absolute_url() if getattr(obj, 'get_absolute_url', None) else None, message, @@ -171,7 +171,7 @@ class Report(object): """ Log a message which is not associated with a particular object. """ - self._log(None, message, level=LOG_DEFAULT) + self._log(None, message, level=LogLevelChoices.LOG_DEFAULT) self.logger.info(message) def log_success(self, obj, message=None): @@ -179,7 +179,7 @@ class Report(object): Record a successful test against an object. Logging a message is optional. """ if message: - self._log(obj, message, level=LOG_SUCCESS) + self._log(obj, message, level=LogLevelChoices.LOG_SUCCESS) self._results[self.active_test]['success'] += 1 self.logger.info(f"Success | {obj}: {message}") @@ -187,7 +187,7 @@ class Report(object): """ Log an informational message. """ - self._log(obj, message, level=LOG_INFO) + self._log(obj, message, level=LogLevelChoices.LOG_INFO) self._results[self.active_test]['info'] += 1 self.logger.info(f"Info | {obj}: {message}") @@ -195,7 +195,7 @@ class Report(object): """ Log a warning. """ - self._log(obj, message, level=LOG_WARNING) + self._log(obj, message, level=LogLevelChoices.LOG_WARNING) self._results[self.active_test]['warning'] += 1 self.logger.info(f"Warning | {obj}: {message}") @@ -203,7 +203,7 @@ class Report(object): """ Log a failure. Calling this method will automatically mark the report as failed. """ - self._log(obj, message, level=LOG_FAILURE) + self._log(obj, message, level=LogLevelChoices.LOG_FAILURE) self._results[self.active_test]['failure'] += 1 self.logger.info(f"Failure | {obj}: {message}") self.failed = True diff --git a/netbox/extras/scripts.py b/netbox/extras/scripts.py index ccb4fdc82..018963bd8 100644 --- a/netbox/extras/scripts.py +++ b/netbox/extras/scripts.py @@ -19,11 +19,10 @@ from mptt.forms import TreeNodeChoiceField, TreeNodeMultipleChoiceField from mptt.models import MPTTModel from extras.api.serializers import ScriptOutputSerializer -from extras.choices import JobResultStatusChoices +from extras.choices import JobResultStatusChoices, LogLevelChoices from extras.models import JobResult from ipam.formfields import IPAddressFormField, IPNetworkFormField from ipam.validators import MaxPrefixLengthValidator, MinPrefixLengthValidator, prefix_validator -from .constants import LOG_DEFAULT, LOG_FAILURE, LOG_INFO, LOG_SUCCESS, LOG_WARNING from utilities.exceptions import AbortTransaction from utilities.forms import DynamicModelChoiceField, DynamicModelMultipleChoiceField from .forms import ScriptForm @@ -324,23 +323,23 @@ class BaseScript: def log_debug(self, message): self.logger.log(logging.DEBUG, message) - self.log.append((LOG_DEFAULT, message)) + self.log.append((LogLevelChoices.LOG_DEFAULT, message)) def log_success(self, message): self.logger.log(logging.INFO, message) # No syslog equivalent for SUCCESS - self.log.append((LOG_SUCCESS, message)) + self.log.append((LogLevelChoices.LOG_SUCCESS, message)) def log_info(self, message): self.logger.log(logging.INFO, message) - self.log.append((LOG_INFO, message)) + self.log.append((LogLevelChoices.LOG_INFO, message)) def log_warning(self, message): self.logger.log(logging.WARNING, message) - self.log.append((LOG_WARNING, message)) + self.log.append((LogLevelChoices.LOG_WARNING, message)) def log_failure(self, message): self.logger.log(logging.ERROR, message) - self.log.append((LOG_FAILURE, message)) + self.log.append((LogLevelChoices.LOG_FAILURE, message)) # Convenience functions @@ -428,11 +427,15 @@ def run_script(data, request, commit=True, *args, **kwargs): try: with transaction.atomic(): script.output = script.run(**kwargs) - job_result.status = JobResultStatusChoices.STATUS_COMPLETED + job_result.data = ScriptOutputSerializer(script).data + job_result.set_status(JobResultStatusChoices.STATUS_COMPLETED) + if not commit: raise AbortTransaction() + except AbortTransaction: pass + except Exception as e: stacktrace = traceback.format_exc() script.log_failure( @@ -440,7 +443,8 @@ def run_script(data, request, commit=True, *args, **kwargs): ) logger.error(f"Exception raised during script execution: {e}") commit = False - job_result.status = JobResultStatusChoices.STATUS_FAILED + job_result.set_status(JobResultStatusChoices.STATUS_ERRORED) + finally: if not commit: # Delete all pending changelog entries @@ -449,10 +453,6 @@ def run_script(data, request, commit=True, *args, **kwargs): "Database changes have been reverted automatically." ) - job_result.data = ScriptOutputSerializer(script).data - job_result.completed = timezone.now() - job_result.save() - logger.info(f"Script completed in {job_result.duration}") # Delete any previous terminal state results diff --git a/netbox/extras/templatetags/log_levels.py b/netbox/extras/templatetags/log_levels.py index 93dbe3897..c92ff8cdf 100644 --- a/netbox/extras/templatetags/log_levels.py +++ b/netbox/extras/templatetags/log_levels.py @@ -1,6 +1,6 @@ from django import template -from extras.constants import LOG_DEFAULT, LOG_FAILURE, LOG_INFO, LOG_SUCCESS, LOG_WARNING +from extras.choices import LogLevelChoices register = template.Library() @@ -11,28 +11,7 @@ def log_level(level): """ Display a label indicating a syslog severity (e.g. info, warning, etc.). """ - # TODO: we should convert this to a choices class - levels = { - 'default': { - 'name': 'Default', - 'class': 'default' - }, - 'success': { - 'name': 'Success', - 'class': 'success', - }, - 'info': { - 'name': 'Info', - 'class': 'info' - }, - 'warning': { - 'name': 'Warning', - 'class': 'warning' - }, - 'failure': { - 'name': 'Failure', - 'class': 'danger' - } + return { + 'name': LogLevelChoices.as_dict()[level], + 'class': dict(LogLevelChoices.CLASS_MAP)[level] } - - return levels[level] diff --git a/netbox/extras/views.py b/netbox/extras/views.py index dfaeba806..a7d89fb1f 100644 --- a/netbox/extras/views.py +++ b/netbox/extras/views.py @@ -453,8 +453,22 @@ class ScriptListView(ContentTypePermissionRequiredMixin, View): def get(self, request): + scripts = get_scripts(use_names=True) + script_content_type = ContentType.objects.get(app_label='extras', model='script') + results = { + r.name: r + for r in JobResult.objects.filter( + obj_type=script_content_type, + status__in=JobResultStatusChoices.TERMINAL_STATE_CHOICES + ).defer('data') + } + + for _scripts in scripts.values(): + for script in _scripts.values(): + script.result = results.get(script.full_name) + return render(request, 'extras/script_list.html', { - 'scripts': get_scripts(use_names=True), + 'scripts': scripts, }) diff --git a/netbox/project-static/js/job_result.js b/netbox/project-static/js/job_result.js index 8f3d360db..581e9d2bc 100644 --- a/netbox/project-static/js/job_result.js +++ b/netbox/project-static/js/job_result.js @@ -3,10 +3,8 @@ var timeout = 1000; function updatePendingStatusLabel(status){ var labelClass; - if (status.value === 'failed'){ + if (status.value === 'failed' || status.value === 'errored'){ labelClass = 'danger'; - } else if (status.value === 'pending'){ - labelClass = 'default'; } else if (status.value === 'running'){ labelClass = 'warning'; } else if (status.value === 'completed'){ @@ -33,7 +31,7 @@ $(document).ready(function(){ context: this, success: function(data) { updatePendingStatusLabel(data.status); - if (data.status.value === 'completed' || data.status.value === 'failed'){ + if (data.status.value === 'completed' || data.status.value === 'failed' || data.status.value === 'errored'){ jobTerminatedAction() } else { setTimeout(checkPendingResult, timeout); diff --git a/netbox/templates/extras/inc/job_label.html b/netbox/templates/extras/inc/job_label.html index 7ae2b353f..cf31c7f75 100644 --- a/netbox/templates/extras/inc/job_label.html +++ b/netbox/templates/extras/inc/job_label.html @@ -1,11 +1,13 @@ {% if result.status == 'failed' %} +{% elif result.status == 'errored' %} + {% elif result.status == 'pending' %} {% elif result.status == 'running' %} {% elif result.status == 'completed' %} - + {% else %} {% endif %} diff --git a/netbox/templates/extras/report_result.html b/netbox/templates/extras/report_result.html index 2053c86df..2433c381d 100644 --- a/netbox/templates/extras/report_result.html +++ b/netbox/templates/extras/report_result.html @@ -38,7 +38,7 @@ {% endif %} {% include 'extras/inc/job_label.html' with result=result %}

- {% if result.completed %} + {% if result.completed and result.status != 'errored' %}
Report Methods @@ -97,8 +97,10 @@
+ {% elif result.status == 'errored' %} +
Error during report execution
{% else %} -
Pending results
+
Pending results
{% endif %}
diff --git a/netbox/templates/extras/script_list.html b/netbox/templates/extras/script_list.html index a1b97cfc2..98803b6a6 100644 --- a/netbox/templates/extras/script_list.html +++ b/netbox/templates/extras/script_list.html @@ -4,15 +4,17 @@ {% block content %}

{% block title %}Scripts{% endblock %}

-
+
{% if scripts %} {% for module, module_scripts in scripts.items %}

{{ module|bettertitle }}

- - + + + + @@ -21,7 +23,15 @@ + + {% if script.result %} + + {% else %} + + {% endif %} {% endfor %} @@ -34,5 +44,26 @@ {% endif %} +
+ {% if scripts %} +
+ {% for module, module_scripts in scripts.items %} +
+ {{ module|bettertitle }} +
+ + {% endfor %} +
+ {% endif %} +
{% endblock %} diff --git a/netbox/templates/extras/script_result.html b/netbox/templates/extras/script_result.html index e34c5a18d..d225a4e10 100644 --- a/netbox/templates/extras/script_result.html +++ b/netbox/templates/extras/script_result.html @@ -41,7 +41,7 @@ {% include 'extras/inc/job_label.html' with result=result %}

- {% if result.completed %} + {% if result.completed and result.status != 'errored' %}
@@ -76,6 +76,12 @@
+ {% elif result.stats == 'errored' %} +
+
+
Error during script execution
+
+
{% else %}
NameDescriptionNameStatusDescriptionLast Run
{{ script }} + {% include 'extras/inc/job_label.html' with result=script.result %} + {{ script.Meta.description }}{{ script.result.created }}Never