Explorar o código

Closes #23311: Avoid building the GraphQL schema during django.setup()

Remove the GraphQL finalizer app, which assembled the complete Strawberry
schema in ready() and imposed ~1-1.5s of startup overhead on every
management command. The schema is now assembled on first import of
netbox.graphql.schema, which runs validate_extension_targets() once
assembly completes. The WSGI entrypoint imports the schema at startup so
that schema errors still prevent workers (and the development server)
from starting.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Jeremy Stretch hai 2 días
pai
achega
80d33790b5

+ 4 - 2
docs/plugins/development/graphql-api.md

@@ -45,8 +45,10 @@ An extension is a mixin class declaring a `models` attribute: a list of the lowe
 
 By default, NetBox imports `type_extensions` and `filter_extensions` from a `graphql_extensions.py` module beside the plugin's `graphql.py`. The `PluginConfig` attributes may override each with a dotted path to a list under any attribute name.
 
+These lists are registered automatically during plugin initialization, which is the only supported way to register extensions. Registering an extension at any later point is not supported: whether its target GraphQL type has already been assembled (and thus whether registration fails) depends on how NetBox was started.
+
 !!! warning
-    Extension modules are imported while plugins initialize, before NetBox's core GraphQL types are assembled. They must not import core GraphQL modules (e.g. `dcim.graphql.types`) at module level. A premature import assembles the affected core types early, and any extension registered afterwards for one of them fails at startup. Reference core types only through `strawberry.lazy()` string annotations. Plugin schema modules (`graphql.py`) are loaded later, during schema assembly, and may import core GraphQL types freely.
+    Extension modules are imported while plugins initialize, before NetBox's core GraphQL types are assembled. They must not import core GraphQL modules (e.g. `dcim.graphql.types`) at module level. A premature import assembles the affected core types early, and any extension registered afterwards for one of them fails once the schema is assembled. Reference core types only through `strawberry.lazy()` string annotations. Plugin schema modules (`graphql.py`) are loaded later, during schema assembly, and may import core GraphQL types freely.
 
 ### Type Extensions
 
@@ -129,7 +131,7 @@ query {
 ```
 
 !!! note
-    Extensions are strictly additive. Every name an extension contributes must be new, not only its GraphQL fields and resolvers but also its helper methods and class attributes. A name the core type already provides, or that two extensions both declare, causes NetBox to fail at startup with an error naming the extension classes. This is deliberate, because Python resolves attribute lookups through the composed MRO, so a shared helper or constant name would let one plugin silently redirect another plugin's resolvers. Two extensions may inherit the same name from one shared helper base, which is not a conflict. Explicit GraphQL aliases are checked as well. Registering an extension after its target GraphQL type has been assembled also raises an error, as does an extension targeting a model that never assembles a GraphQL type or filter. Extensions are plain mixin types and may not implement GraphQL interfaces or inherit from core GraphQL classes.
+    Extensions are strictly additive. Every name an extension contributes must be new, not only its GraphQL fields and resolvers but also its helper methods and class attributes. A name the core type already provides, or that two extensions both declare, causes schema assembly to fail with an error naming the extension classes. This is deliberate, because Python resolves attribute lookups through the composed MRO, so a shared helper or constant name would let one plugin silently redirect another plugin's resolvers. Two extensions may inherit the same name from one shared helper base, which is not a conflict. Explicit GraphQL aliases are checked as well. Registering an extension after its target GraphQL type has been assembled also raises an error, as does an extension targeting a model that never assembles a GraphQL type or filter. Extensions are plain mixin types and may not implement GraphQL interfaces or inherit from core GraphQL classes.
 
 ## GraphQL Objects
 

+ 0 - 13
netbox/netbox/graphql/apps.py

@@ -1,13 +0,0 @@
-from django.apps import AppConfig
-
-
-class GraphQLConfig(AppConfig):
-    name = 'netbox.graphql'
-    label = 'netbox_graphql'
-
-    def ready(self):
-        # Runs after every plugin's ready(), so schema errors fail django.setup() instead of the first request.
-        from netbox.graphql import schema  # noqa: F401
-        from netbox.graphql.utils import validate_extension_targets
-
-        validate_extension_targets()

+ 4 - 0
netbox/netbox/graphql/schema.py

@@ -20,6 +20,7 @@ from vpn.graphql.schema import VPNQuery
 from wireless.graphql.schema import WirelessQuery
 
 from .scalars import BigInt, BigIntScalar
+from .utils import validate_extension_targets
 
 SchemaExtensionFactory = type[SchemaExtension] | Callable[[], SchemaExtension]
 
@@ -68,3 +69,6 @@ schema = strawberry.Schema(
     ),
     extensions=get_schema_extensions(),
 )
+
+# Must run after schema assembly, since extension targets are recorded as their types are assembled.
+validate_extension_targets()

+ 2 - 2
netbox/netbox/graphql/utils.py

@@ -164,7 +164,7 @@ def validate_extension_final_names(core_type, extensions):
 def validate_extension_targets():
     """
     Reject extensions whose target model never assembled a GraphQL type or filter, since they would otherwise
-    be silently discarded. Runs from the finalizer app after schema assembly.
+    be silently discarded. Runs from netbox.graphql.schema after schema assembly.
     """
     assembled = registry['plugins']['graphql_extensions_assembled']
     for store in ('graphql_type_extensions', 'graphql_filter_extensions'):
@@ -180,7 +180,7 @@ def validate_extension_targets():
 def register_model_graphql_type(model, delegate, store_key, **kwargs):
     """
     Decorator factory composing registered plugin extensions into a core GraphQL type or filter class. The
-    finalizer app assembles the schema during django.setup(), after every plugin has initialized, and
+    schema is assembled on first import of netbox.graphql.schema, after every plugin has initialized, and
     registering an extension once its target has assembled raises through the assembled-target set.
     """
     label = get_model_label(model)

+ 0 - 3
netbox/netbox/settings.py

@@ -1034,9 +1034,6 @@ for plugin_name in PLUGINS:
         else:
             raise ImproperlyConfigured(f"events_pipline in plugin: {plugin_name} must be a list or tuple")
 
-# GraphQL assembly must run after every plugin has initialized.
-INSTALLED_APPS.append('netbox.graphql.apps.GraphQLConfig')
-
 
 #
 # Monkey-patching

+ 42 - 15
netbox/netbox/tests/test_plugins.py

@@ -258,37 +258,64 @@ class PluginTestCase(TestCase):
             with self.assertRaises(ModuleNotFoundError):
                 config._load_resource('graphql_type_extensions')
 
-    def test_graphql_finalizer_app_installed_after_plugins(self):
-        finalizer = settings.INSTALLED_APPS.index('netbox.graphql.apps.GraphQLConfig')
-        plugin_positions = [i for i, app in enumerate(settings.INSTALLED_APPS) if 'dummy_plugin' in app]
-        self.assertTrue(plugin_positions)
-        self.assertGreater(finalizer, max(plugin_positions))
-        self.assertEqual(apps.get_app_config('netbox_graphql').name, 'netbox.graphql')
-
-    def test_graphql_finalizer_runs_during_django_setup(self):
-        """The finalizer assembles the schema before auditing targets, and its errors fail django.setup()."""
+    def _run_child(self, script):
+        # The child inherits this process's settings, which is safe only because the script touches no database.
+        return subprocess.run(
+            [sys.executable, '-c', script], capture_output=True, text=True, cwd=settings.BASE_DIR, timeout=300
+        )
+
+    def test_graphql_schema_deferred_until_wsgi_load(self):
+        """
+        django.setup() assembles no GraphQL types, so management commands which don't need the schema skip the cost,
+        while loading the WSGI application loads the URLconf and assembles the schema.
+        """
         # Source for a child interpreter, so it carries no indentation of its own.
         script = """
 import sys
 import django
+from netbox.registry import registry
+
+django.setup()
+if 'netbox.graphql.schema' in sys.modules:
+    raise RuntimeError('SCHEMA_IMPORTED_DURING_SETUP')
+if registry['plugins']['graphql_extensions_assembled']:
+    raise RuntimeError('TYPES_ASSEMBLED_DURING_SETUP')
+
+import netbox.wsgi
+from django.urls import get_resolver
+
+if 'url_patterns' not in vars(get_resolver()):
+    raise RuntimeError('URLCONF_NOT_LOADED')
+if not hasattr(sys.modules.get('netbox.graphql.schema'), 'schema'):
+    raise RuntimeError('SCHEMA_NOT_ASSEMBLED')
+"""
+        result = self._run_child(script)
+        self.assertEqual(result.returncode, 0, f"stdout:\n{result.stdout}\n\nstderr:\n{result.stderr}")
+
+    def test_wsgi_application_audits_targets_after_assembly(self):
+        """
+        Loading the WSGI application assembles the schema before auditing extension targets, and audit errors
+        propagate so that a broken extension prevents the WSGI application from loading.
+        """
+        script = """
+import sys
 from netbox.graphql import utils
 
 
 def audit():
-    if 'netbox.graphql.schema' not in sys.modules:
+    if not hasattr(sys.modules['netbox.graphql.schema'], 'schema'):
         raise RuntimeError('SCHEMA_NOT_ASSEMBLED')
     raise RuntimeError('AUDIT_RAN_AFTER_SCHEMA')
 
 
 utils.validate_extension_targets = audit
-django.setup()
+import netbox.wsgi
+print('WORKER_STARTED')
 """
-        # The child inherits this process's settings, which is safe only because ready() touches no database.
-        result = subprocess.run(
-            [sys.executable, '-c', script], capture_output=True, text=True, cwd=settings.BASE_DIR, timeout=300
-        )
+        result = self._run_child(script)
         self.assertNotEqual(result.returncode, 0, f"stdout:\n{result.stdout}\n\nstderr:\n{result.stderr}")
         self.assertIn('AUDIT_RAN_AFTER_SCHEMA', result.stderr)
+        self.assertNotIn('WORKER_STARTED', result.stdout)
 
     def test_missing_plugin_app_config_raises_clear_error(self):
         installed = [*registry['plugins']['installed'], 'not_a_real_plugin']

+ 6 - 0
netbox/netbox/wsgi.py

@@ -1,7 +1,13 @@
 import os
 
 from django.core.wsgi import get_wsgi_application
+from django.urls import get_resolver
 
 os.environ.setdefault("DJANGO_SETTINGS_MODULE", "netbox.settings")
 
 application = get_wsgi_application()
+
+# Load the URLconf (and with it the GraphQL schema) at startup, so URLconf, plugin URL, and schema errors prevent the
+# worker from starting instead of failing the first request. (This is skipped for management commands which don't
+# need the URLconf.)
+get_resolver().url_patterns

+ 10 - 0
netbox/utilities/rqworker.py

@@ -1,5 +1,6 @@
 import logging
 
+from django.urls import get_resolver
 from django_rq.queues import get_connection
 from rq import Retry, Worker
 from rq.worker_registration import REDIS_WORKER_KEYS
@@ -27,8 +28,17 @@ class NetBoxRQWorker(Worker):
     was lost and rebuilt while the worker was running), the next heartbeat
     will re-register the worker so that Worker.all() / Worker.find_by_key()
     can locate it again.
+
+    The URLconf (and with it the GraphQL schema) is loaded once on startup, so
+    that forked work horses inherit it rather than each rebuilding it.
     """
 
+    def bootstrap(self, *args, **kwargs):
+        # Load the URLconf before registering the worker, so that a broken URLconf or GraphQL schema prevents the
+        # worker from starting rather than failing every job.
+        get_resolver().url_patterns
+        super().bootstrap(*args, **kwargs)
+
     def heartbeat(self, *args, **kwargs):
         try:
             if not self.connection.sismember(REDIS_WORKER_KEYS, self.key):

+ 35 - 1
netbox/utilities/tests/test_rqworker.py

@@ -1,6 +1,7 @@
 import logging
-from unittest.mock import MagicMock, patch
+from unittest.mock import MagicMock, PropertyMock, patch
 
+from django.core.exceptions import ImproperlyConfigured
 from django.test import TestCase
 
 from utilities.rqworker import (
@@ -110,6 +111,39 @@ class NetBoxRQWorkerHeartbeatTestCase(TestCase):
         super_heartbeat.assert_called_once()
 
 
+class NetBoxRQWorkerBootstrapTestCase(TestCase):
+    """
+    The overridden bootstrap() must load the URLconf before invoking
+    super().bootstrap(), so that forked work horses inherit it.
+    """
+
+    def _make_resolver(self, side_effect=None):
+        resolver = MagicMock()
+        url_patterns = PropertyMock(side_effect=side_effect)
+        type(resolver).url_patterns = url_patterns
+        return resolver, url_patterns
+
+    def test_bootstrap_loads_urlconf_before_super(self):
+        worker = NetBoxRQWorker.__new__(NetBoxRQWorker)
+        resolver, url_patterns = self._make_resolver()
+        loaded_before_super = []
+        with patch('utilities.rqworker.get_resolver', return_value=resolver), \
+                patch('rq.Worker.bootstrap') as super_bootstrap:
+            super_bootstrap.side_effect = lambda *args, **kwargs: loaded_before_super.append(url_patterns.called)
+            NetBoxRQWorker.bootstrap(worker, 'DEBUG', date_format='%H')
+        self.assertEqual(loaded_before_super, [True])
+        super_bootstrap.assert_called_once_with('DEBUG', date_format='%H')
+
+    def test_bootstrap_urlconf_error_aborts_before_registration(self):
+        worker = NetBoxRQWorker.__new__(NetBoxRQWorker)
+        resolver, _ = self._make_resolver(side_effect=ImproperlyConfigured('broken schema'))
+        with patch('utilities.rqworker.get_resolver', return_value=resolver), \
+                patch('rq.Worker.bootstrap') as super_bootstrap:
+            with self.assertRaises(ImproperlyConfigured):
+                NetBoxRQWorker.bootstrap(worker)
+        super_bootstrap.assert_not_called()
+
+
 class GetWorkersForQueueTestCase(TestCase):
     """
     get_workers_for_queue() must: