From 107c46cb7a5c4787c688271032c9e737bd46265b Mon Sep 17 00:00:00 2001 From: jeremystretch Date: Fri, 24 Mar 2023 11:07:53 -0400 Subject: [PATCH] Remove get_modules() utility function --- netbox/extras/api/views.py | 28 ++-- .../extras/management/commands/runreport.py | 14 +- netbox/extras/reports.py | 20 +-- netbox/extras/scripts.py | 18 +-- netbox/extras/utils.py | 47 ------ netbox/extras/views.py | 66 ++++---- netbox/templates/extras/report_list.html | 142 +++++++++--------- netbox/templates/extras/script_list.html | 96 ++++++------ 8 files changed, 170 insertions(+), 261 deletions(-) diff --git a/netbox/extras/api/views.py b/netbox/extras/api/views.py index dfee44a51..d09c88abe 100644 --- a/netbox/extras/api/views.py +++ b/netbox/extras/api/views.py @@ -16,8 +16,8 @@ from extras import filtersets from extras.choices import JobResultStatusChoices from extras.models import * from extras.models import CustomField -from extras.reports import get_report, get_reports, run_report -from extras.scripts import get_script, get_scripts, run_script +from extras.reports import get_report, run_report +from extras.scripts import get_script, run_script from netbox.api.authentication import IsAuthenticatedOrLoginNotRequired from netbox.api.features import SyncedDataMixin from netbox.api.metadata import ContentTypeMetadata @@ -27,7 +27,6 @@ from utilities.exceptions import RQWorkerNotRunningException from utilities.utils import copy_safe_request, count_related from . import serializers from .mixins import ConfigTemplateRenderMixin -from .nested_serializers import NestedConfigTemplateSerializer class ExtrasRootView(APIRootView): @@ -189,7 +188,6 @@ class ReportViewSet(ViewSet): """ Compile all reports and their related results (if any). Result data is deferred in the list view. """ - report_list = [] report_content_type = ContentType.objects.get(app_label='extras', model='report') results = { r.name: r @@ -199,13 +197,13 @@ class ReportViewSet(ViewSet): ).order_by('name', '-created').distinct('name').defer('data') } - # Iterate through all available Reports. - for module_name, reports in get_reports().items(): - for report in reports.values(): + report_list = [] + for report_module in ReportModule.objects.restrict(request.user): + report_list.extend([report() for report in report_module.reports.values()]) - # Attach the relevant JobResult (if any) to each Report. - report.result = results.get(report.full_name, None) - report_list.append(report) + # Attach JobResult objects to each report (if any) + for report in report_list: + report.result = results.get(report.full_name, None) serializer = serializers.ReportSerializer(report_list, many=True, context={ 'request': request, @@ -296,15 +294,15 @@ class ScriptViewSet(ViewSet): ).order_by('name', '-created').distinct('name').defer('data') } - flat_list = [] - for script_list in get_scripts().values(): - flat_list.extend(script_list.values()) + script_list = [] + for script_module in ScriptModule.objects.restrict(request.user): + script_list.extend(script_module.scripts.values()) # Attach JobResult objects to each script (if any) - for script in flat_list: + for script in script_list: script.result = results.get(script.full_name, None) - serializer = serializers.ScriptSerializer(flat_list, many=True, context={'request': request}) + serializer = serializers.ScriptSerializer(script_list, many=True, context={'request': request}) return Response(serializer.data) diff --git a/netbox/extras/management/commands/runreport.py b/netbox/extras/management/commands/runreport.py index 38d435613..9a61e7805 100644 --- a/netbox/extras/management/commands/runreport.py +++ b/netbox/extras/management/commands/runreport.py @@ -5,8 +5,8 @@ from django.core.management.base import BaseCommand from django.utils import timezone from extras.choices import JobResultStatusChoices -from extras.models import JobResult -from extras.reports import get_reports, run_report +from extras.models import JobResult, ReportModule +from extras.reports import run_report class Command(BaseCommand): @@ -17,13 +17,9 @@ class Command(BaseCommand): def handle(self, *args, **options): - # Gather all available reports - reports = get_reports() - - # Run reports - for module_name, report_list in reports.items(): - for report in report_list.values(): - if module_name in options['reports'] or report.full_name in options['reports']: + for module in ReportModule.objects.all(): + for report in module.reports.values(): + if module.name in options['reports'] or report.full_name in options['reports']: # Run the report and create a new JobResult self.stdout.write( diff --git a/netbox/extras/reports.py b/netbox/extras/reports.py index c028a0a98..90ffe0cc5 100644 --- a/netbox/extras/reports.py +++ b/netbox/extras/reports.py @@ -7,32 +7,16 @@ from django_rq import job from .choices import JobResultStatusChoices, LogLevelChoices from .models import JobResult, ReportModule -from .temp import is_report -from .utils import get_modules logger = logging.getLogger(__name__) -def get_reports(): - return get_modules(ReportModule.objects.all(), is_report, 'report_order') - - def get_report(module_name, report_name): """ Return a specific report from within a module. """ - reports = get_reports() - module = reports.get(module_name) - - if module is None: - return None - - report = module.get(report_name) - - if report is None: - return None - - return report + module = ReportModule.objects.get(file_path=f'{module_name}.py') + return module.scripts.get(report_name) @job('default') diff --git a/netbox/extras/scripts.py b/netbox/extras/scripts.py index 347438f9f..eb73f8b51 100644 --- a/netbox/extras/scripts.py +++ b/netbox/extras/scripts.py @@ -22,8 +22,6 @@ from utilities.exceptions import AbortScript, AbortTransaction from utilities.forms import add_blank_choice, DynamicModelChoiceField, DynamicModelMultipleChoiceField from .context_managers import change_logging from .forms import ScriptForm -from .temp import is_script -from .utils import get_modules __all__ = [ 'BaseScript', @@ -432,10 +430,6 @@ def is_variable(obj): return isinstance(obj, ScriptVariable) -def get_scripts(): - return get_modules(ScriptModule.objects.all(), is_script, 'script_order') - - 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 @@ -444,10 +438,10 @@ def run_script(data, request, commit=True, *args, **kwargs): job_result = kwargs.pop('job_result') job_result.start() - module, script_name = job_result.name.split('.', 1) - script = get_script(module, script_name)() + module_name, script_name = job_result.name.split('.', 1) + script = get_script(module_name, script_name)() - logger = logging.getLogger(f"netbox.scripts.{module}.{script_name}") + logger = logging.getLogger(f"netbox.scripts.{module_name}.{script_name}") logger.info(f"Running script (commit={commit})") # Add files to form data @@ -518,7 +512,5 @@ def get_script(module_name, script_name): """ Retrieve a script class by module and name. Returns None if the script does not exist. """ - scripts = get_scripts() - module = scripts.get(module_name) - if module: - return module.get(script_name) + module = ScriptModule.objects.get(file_path=f'{module_name}.py') + return module.scripts.get(script_name) diff --git a/netbox/extras/utils.py b/netbox/extras/utils.py index 073f1d13f..70c9427d6 100644 --- a/netbox/extras/utils.py +++ b/netbox/extras/utils.py @@ -1,5 +1,3 @@ -import inspect -import sys import threading from django.db.models import Q @@ -72,48 +70,3 @@ def register_features(model, features): raise KeyError( f"{feature} is not a valid model feature! Valid keys are: {registry['model_features'].keys()}" ) - - -def get_modules(queryset, litmus_func, ordering_attr): - """ - Returns a list of tuples: - - [ - (module, (child, child, ...)), - (module, (child, child, ...)), - ... - ] - """ - results = {} - - modules = [(mf, *mf.get_module_info()) for mf in queryset] - modules_bases = set([name.split(".")[0] for _, _, name, _ in modules]) - - # Deleting from sys.modules needs to done behind a lock to prevent race conditions where a module is - # removed from sys.modules while another thread is importing - with lock: - for module_name in list(sys.modules.keys()): - # Everything sharing a base module path with a module in the script folder is removed. - # We also remove all modules with a base module called "scripts". This allows modifying imported - # non-script modules without having to reload the RQ worker. - module_base = module_name.split(".")[0] - if module_base in ('reports', 'scripts', *modules_bases): - del sys.modules[module_name] - - for mf, importer, module_name, _ in modules: - module = importer.find_module(module_name).load_module(module_name) - child_order = getattr(module, ordering_attr, ()) - ordered_children = [cls() for cls in child_order if litmus_func(cls)] - unordered_children = [cls() for _, cls in inspect.getmembers(module, litmus_func) if cls not in child_order] - - children = {} - - for cls in [*ordered_children, *unordered_children]: - # For child objects in submodules use the full import path w/o the root module as the name - child_name = cls.full_name.split(".", maxsplit=1)[1] - children[child_name] = cls - - if children: - results[mf] = children - - return results diff --git a/netbox/extras/views.py b/netbox/extras/views.py index 8495914ba..19b1ba7c3 100644 --- a/netbox/extras/views.py +++ b/netbox/extras/views.py @@ -22,8 +22,8 @@ from .choices import JobResultStatusChoices from .constants import SCRIPTS_ROOT_NAME, REPORTS_ROOT_NAME from .forms.reports import ReportForm from .models import * -from .reports import get_report, get_reports, run_report -from .scripts import get_scripts, run_script +from .reports import get_report, run_report +from .scripts import run_script # @@ -809,16 +809,16 @@ class ReportModuleDeleteView(generic.ObjectDeleteView): class ReportListView(ContentTypePermissionRequiredMixin, View): """ - Retrieve all of the available reports from disk and the recorded JobResult (if any) for each. + Retrieve all the available reports from disk and the recorded JobResult (if any) for each. """ def get_required_permission(self): return 'extras.view_report' def get(self, request): + report_modules = ReportModule.objects.restrict(request.user) - reports = get_reports() report_content_type = ContentType.objects.get(app_label='extras', model='report') - results = { + job_results = { r.name: r for r in JobResult.objects.filter( obj_type=report_content_type, @@ -826,18 +826,17 @@ class ReportListView(ContentTypePermissionRequiredMixin, View): ).order_by('name', '-created').distinct('name').defer('data') } - ret = [] - - for module, report_list in reports.items(): - module_reports = [] - for report in report_list.values(): - report.result = results.get(report.full_name, None) - module_reports.append(report) - ret.append((module, module_reports)) + # for module, report_list in reports.items(): + # module_reports = [] + # for report in report_list.values(): + # report.result = results.get(report.full_name, None) + # module_reports.append(report) + # ret.append((module, module_reports)) return render(request, 'extras/report_list.html', { - 'model': ReportModule, - 'reports': ret, + 'model': ScriptModule, + 'report_modules': report_modules, + 'job_results': job_results, }) @@ -955,27 +954,16 @@ class ScriptModuleDeleteView(generic.ObjectDeleteView): queryset = ScriptModule.objects.all() -class GetScriptMixin: - def _get_script(self, name, module=None): - if module is None: - module, name = name.split('.', 1) - scripts = get_scripts() - try: - return scripts[module][name]() - except KeyError: - raise Http404 - - class ScriptListView(ContentTypePermissionRequiredMixin, View): def get_required_permission(self): return 'extras.view_script' def get(self, request): + script_modules = ScriptModule.objects.restrict(request.user) - scripts = get_scripts() script_content_type = ContentType.objects.get(app_label='extras', model='script') - results = { + job_results = { r.name: r for r in JobResult.objects.filter( obj_type=script_content_type, @@ -983,17 +971,18 @@ class ScriptListView(ContentTypePermissionRequiredMixin, View): ).order_by('name', '-created').distinct('name').defer('data') } - for _scripts in scripts.values(): - for script in _scripts.values(): - script.result = results.get(script.full_name) + # for _scripts in scripts.values(): + # for script in _scripts.values(): + # script.result = results.get(script.full_name) return render(request, 'extras/script_list.html', { 'model': ScriptModule, - 'scripts': scripts, + 'script_modules': script_modules, + 'job_results': job_results, }) -class ScriptView(ContentTypePermissionRequiredMixin, GetScriptMixin, View): +class ScriptView(ContentTypePermissionRequiredMixin, View): def get_required_permission(self): return 'extras.view_script' @@ -1018,12 +1007,11 @@ class ScriptView(ContentTypePermissionRequiredMixin, GetScriptMixin, View): }) def post(self, request, module, name): - - # Permissions check if not request.user.has_perm('extras.run_script'): return HttpResponseForbidden() - script = self._get_script(name, module) + module = get_object_or_404(ScriptModule.objects.restrict(request.user), file_path=f'{module}.py') + script = module.scripts[name]() form = script.as_form(request.POST, request.FILES) # Allow execution only if RQ worker process is running @@ -1053,7 +1041,7 @@ class ScriptView(ContentTypePermissionRequiredMixin, GetScriptMixin, View): }) -class ScriptResultView(ContentTypePermissionRequiredMixin, GetScriptMixin, View): +class ScriptResultView(ContentTypePermissionRequiredMixin, View): def get_required_permission(self): return 'extras.view_script' @@ -1064,7 +1052,9 @@ class ScriptResultView(ContentTypePermissionRequiredMixin, GetScriptMixin, View) if result.obj_type != script_content_type: raise Http404 - script = self._get_script(result.name) + module_name, script_name = result.name.split('.', 1) + module = get_object_or_404(ScriptModule.objects.restrict(request.user), file_path=f'{module_name}.py') + script = module.scripts[script_name]() # If this is an HTMX request, return only the result HTML if is_htmx(request): diff --git a/netbox/templates/extras/report_list.html b/netbox/templates/extras/report_list.html index 0798455fe..72eb3b43d 100644 --- a/netbox/templates/extras/report_list.html +++ b/netbox/templates/extras/report_list.html @@ -24,91 +24,89 @@ {% block content-wrapper %}
- {% if reports %} - {% for module, module_reports in reports %} -
-
- {% if perms.extras.delete_reportmodule %} -
- - Delete - -
- {% endif %} - - {{ module.name|bettertitle }} -
-
- - + {% for module in report_modules %} +
+
+ {% if perms.extras.delete_reportmodule %} +
+ + Delete + +
+ {% endif %} + + {{ module.name|bettertitle }} +
+
+
+ + + + + + + + + + + {% for report_name, report in module.reports.items %} - - - - - + + + + + - - - {% for report in module_reports %} + {% for method, stats in report.result.data.items %} - - - - - - {% for method, stats in report.result.data.items %} - - - - - {% endfor %} {% endfor %} - -
NameStatusDescriptionLast Run
NameStatusDescriptionLast Run + {{ report.name }} + + {% include 'extras/inc/job_label.html' with result=report.result %} + {{ report.description|markdown|placeholder }} + {% if report.result %} + {{ report.result.created|annotated_date }} + {% else %} + Never + {% endif %} + + {% if perms.extras.run_report %} +
+
+ {% csrf_token %} + +
+
+ {% endif %} +
- {{ report.name }} + + {{ method }} - {% include 'extras/inc/job_label.html' with result=report.result %} - {{ report.description|markdown|placeholder }} - {% if report.result %} - {{ report.result.created|annotated_date }} - {% else %} - Never - {% endif %} - - {% if perms.extras.run_report %} -
-
- {% csrf_token %} - -
-
- {% endif %} + 		+ {{ stats.success }} + {{ stats.info }} + {{ stats.warning }} + {{ stats.failure }}
- {{ method }} - - {{ stats.success }} - {{ stats.info }} - {{ stats.warning }} - {{ stats.failure }} -
-
+ {% endfor %} + +
- {% endfor %} - {% else %} +
+ {% empty %} - {% endif %} + {% endfor %} {% endblock content-wrapper %} diff --git a/netbox/templates/extras/script_list.html b/netbox/templates/extras/script_list.html index 4df657fec..171a02e8b 100644 --- a/netbox/templates/extras/script_list.html +++ b/netbox/templates/extras/script_list.html @@ -23,63 +23,61 @@ {% block content-wrapper %}
- {% if scripts %} - {% for module, module_scripts in scripts.items %} -
-
- {% if perms.extras.delete_scriptmodule %} -
- - Delete - -
- {% endif %} - - {{ module.name|bettertitle }} -
-
- - + {% for module in script_modules %} +
+
+ {% if perms.extras.delete_scriptmodule %} +
+ + Delete + +
+ {% endif %} + + {{ module.name|bettertitle }} +
+
+
+ + + + + + + + + + {% for script_name, script_class in module.scripts.items %} - - - - + + + + {% if script_class.result %} + + {% else %} + + {% endif %} - - - {% for class_name, script in module_scripts.items %} - - - - - {% if script.result %} - - {% else %} - - {% endif %} - - {% endfor %} - -
NameStatusDescriptionLast Run
NameStatusDescriptionLast Run + {{ script_class.name }} + + {% include 'extras/inc/job_label.html' with result=script_class.result %} + + {{ script_class.Meta.description|markdown|placeholder }} + + {{ script_class.result.created|annotated_date }} + Never
- {{ script.name }} - - {% include 'extras/inc/job_label.html' with result=script.result %} - - {{ script.Meta.description|markdown|placeholder }} - - {{ script.result.created|annotated_date }} - Never
-
+ {% endfor %} + +
- {% endfor %} - {% else %} +
+ {% empty %}

No Scripts Found

Scripts should be saved to {{ settings.SCRIPTS_ROOT }}.
This path can be changed by setting SCRIPTS_ROOT in NetBox's configuration.
- {% endif %} + {% endfor %} {% endblock content-wrapper %}