Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't import wagtailmedia models if it's not in INSTALLED_APPS #235

Merged
merged 1 commit into from
Jul 25, 2022

Conversation

kaedroho
Copy link
Contributor

If you have wagtailmedia installed with pip, but not in INSTALLED_APPS, Django will raise an error on startup:

Traceback (most recent call last):
  File "/home/gitpod/.pyenv/versions/3.8.12/lib/python3.8/threading.py", line 932, in _bootstrap_inner
    self.run()
  File "/home/gitpod/.pyenv/versions/3.8.12/lib/python3.8/threading.py", line 870, in run
    self._target(*self._args, **self._kwargs)
  File "/workspace/.pip-modules/lib/python3.8/site-packages/django/utils/autoreload.py", line 64, in wrapper
    fn(*args, **kwargs)
  File "/workspace/.pip-modules/lib/python3.8/site-packages/django/core/management/commands/runserver.py", line 110, in inner_run
    autoreload.raise_last_exception()
  File "/workspace/.pip-modules/lib/python3.8/site-packages/django/utils/autoreload.py", line 87, in raise_last_exception
    raise _exception[1]
  File "/workspace/.pip-modules/lib/python3.8/site-packages/django/core/management/__init__.py", line 375, in execute
    autoreload.check_errors(django.setup)()
  File "/workspace/.pip-modules/lib/python3.8/site-packages/django/utils/autoreload.py", line 64, in wrapper
    fn(*args, **kwargs)
  File "/workspace/.pip-modules/lib/python3.8/site-packages/django/__init__.py", line 24, in setup
    apps.populate(settings.INSTALLED_APPS)
  File "/workspace/.pip-modules/lib/python3.8/site-packages/django/apps/registry.py", line 122, in populate
    app_config.ready()
  File "/workspace/gitpod-wagtail-develop/wagtail-grapple/grapple/apps.py", line 12, in ready
    from .actions import import_apps, load_type_fields
  File "/workspace/gitpod-wagtail-develop/wagtail-grapple/grapple/actions.py", line 42, in <module>
    from wagtailmedia.models import AbstractMedia
  File "/workspace/.pip-modules/lib/python3.8/site-packages/wagtailmedia/models.py", line 157, in <module>
    class Media(AbstractMedia):
  File "/workspace/.pip-modules/lib/python3.8/site-packages/django/db/models/base.py", line 113, in __new__
    raise RuntimeError(
RuntimeError: Model class wagtailmedia.models.Media doesn't declare an explicit app_label and isn't in an application in INSTALLED_APPS.

This PR adds a check to see if wagtailmedia is in INSTALLED_APPS before trying to import its models.

@kaedroho kaedroho requested a review from zerolab July 25, 2022 13:19
@kaedroho kaedroho changed the title Don't import wagtailmedia models unless it's installed Don't import wagtailmedia models if it's not in INSTALLED_APPS Jul 25, 2022
Copy link
Member

@zerolab zerolab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a couple more places that would be good to add extra guards:

diff --git a/grapple/models.py b/grapple/models.py
index 4b57479..706739b 100644
--- a/grapple/models.py
+++ b/grapple/models.py
@@ -121,15 +121,6 @@ def GraphQLDocument(field_name: str, **kwargs):
     return Mixin
 
 
-def GraphQLMedia(field_name: str, **kwargs):
-    def Mixin():
-        from .types.media import get_media_type
-
-        return GraphQLField(field_name, get_media_type, **kwargs)
-
-    return Mixin
-
-
 def GraphQLPage(field_name: str, **kwargs):
     def Mixin():
         from .types.pages import PageInterface
@@ -211,3 +202,13 @@ def GraphQLTag(field_name: str, **kwargs):
         return GraphQLField(field_name, TagObjectType, **kwargs)
 
     return Mixin
+
+
+if apps.is_installed("wagtailmedia"):
+    def GraphQLMedia(field_name: str, **kwargs):
+        def Mixin():
+            from .types.media import get_media_type
+
+            return GraphQLField(field_name, get_media_type, **kwargs)
+
+        return Mixin
\ No newline at end of file
diff --git a/grapple/wagtail_hooks.py b/grapple/wagtail_hooks.py
index b7049fc..d5e9d57 100644
--- a/grapple/wagtail_hooks.py
+++ b/grapple/wagtail_hooks.py
@@ -1,3 +1,4 @@
+from django.apps import apps
 from graphene import ObjectType
 
 try:
@@ -34,12 +35,10 @@ def register_schema_query(query_mixins):
         RedirectsQuery,
     ]
 
-    try:
+    if apps.is_installed("wagtailmedia"):
         from .types.media import MediaQuery
 
         query_mixins.append(MediaQuery())
-    except ModuleNotFoundError:
-        pass
 
     query_mixins += registry.schema
 

@zerolab zerolab merged commit ea0ae10 into torchbox:main Jul 25, 2022
@zerolab
Copy link
Member

zerolab commented Jul 25, 2022

Applied the changes and merge manually. Thanks @kaedroho

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants