Browse Source

Removed the only_with_models_module argument of get_model[s].

Now that the refactorings are complete, it isn't particularly useful any
more, nor very well named. Let's keep the API as simple as possible.

Fixed #21689.
Aymeric Augustin 11 years ago
parent
commit
bfcc686d22

+ 14 - 21
django/apps/registry.py

@@ -101,35 +101,24 @@ class Apps(object):
                 "App registry isn't populated yet. "
                 "Have you called django.setup()?")
 
-    def get_app_configs(self, only_with_models_module=False):
+    def get_app_configs(self):
         """
         Imports applications and returns an iterable of app configs.
-
-        If only_with_models_module in True (non-default), imports models and
-        considers only applications containing a models module.
         """
         self.check_ready()
-        for app_config in self.app_configs.values():
-            if only_with_models_module and app_config.models_module is None:
-                continue
-            yield app_config
+        return self.app_configs.values()
 
-    def get_app_config(self, app_label, only_with_models_module=False):
+    def get_app_config(self, app_label):
         """
         Imports applications and returns an app config for the given label.
 
         Raises LookupError if no application exists with this label.
-
-        If only_with_models_module in True (non-default), imports models and
-        considers only applications containing a models module.
         """
         self.check_ready()
-        app_config = self.app_configs.get(app_label)
-        if app_config is None:
+        try:
+            return self.app_configs[app_label]
+        except KeyError:
             raise LookupError("No installed app with label '%s'." % app_label)
-        if only_with_models_module and app_config.models_module is None:
-            raise LookupError("App '%s' doesn't have a models module." % app_label)
-        return app_config
 
     # This method is performance-critical at least for Django's test suite.
     @lru_cache.lru_cache(maxsize=None)
@@ -319,11 +308,14 @@ class Apps(object):
             "get_app_config(app_label).models_module supersedes get_app(app_label).",
             PendingDeprecationWarning, stacklevel=2)
         try:
-            return self.get_app_config(
-                app_label, only_with_models_module=True).models_module
+            models_module = self.get_app_config(app_label).models_module
         except LookupError as exc:
             # Change the exception type for backwards compatibility.
             raise ImproperlyConfigured(*exc.args)
+        if models_module is None:
+            raise ImproperlyConfigured(
+                "App '%s' doesn't have a models module." % app_label)
+        return models_module
 
     def get_apps(self):
         """
@@ -332,8 +324,9 @@ class Apps(object):
         warnings.warn(
             "[a.models_module for a in get_app_configs()] supersedes get_apps().",
             PendingDeprecationWarning, stacklevel=2)
-        app_configs = self.get_app_configs(only_with_models_module=True)
-        return [app_config.models_module for app_config in app_configs]
+        app_configs = self.get_app_configs()
+        return [app_config.models_module for app_config in app_configs
+                if app_config.models_module is not None]
 
     def _get_app_package(self, app):
         return '.'.join(app.__name__.split('.')[:-1])

+ 1 - 1
django/contrib/contenttypes/management.py

@@ -88,7 +88,7 @@ If you're unsure, answer 'no'.
 
 
 def update_all_contenttypes(**kwargs):
-    for app_config in apps.get_app_configs(only_with_models_module=True):
+    for app_config in apps.get_app_configs():
         update_contenttypes(app_config, **kwargs)
 
 

+ 0 - 3
django/core/management/base.py

@@ -344,9 +344,6 @@ class AppCommand(BaseCommand):
         from django.apps import apps
         if not app_labels:
             raise CommandError("Enter at least one application label.")
-        # Don't use only_with_models_module=True in get_app_config() to tell
-        # apart missing apps from apps without a model module -- which can't
-        # be supported with the legacy API since it passes the models module.
         try:
             app_configs = [apps.get_app_config(app_label) for app_label in app_labels]
         except (LookupError, ImportError) as e:

+ 2 - 2
django/core/management/commands/dumpdata.py

@@ -82,8 +82,8 @@ class Command(BaseCommand):
             if primary_keys:
                 raise CommandError("You can only use --pks option with one model")
             app_list = OrderedDict((app_config, None)
-                for app_config in apps.get_app_configs(only_with_models_module=True)
-                if app_config not in excluded_apps)
+                for app_config in apps.get_app_configs()
+                if app_config.models_module is not None and app_config not in excluded_apps)
         else:
             if len(app_labels) > 1 and primary_keys:
                 raise CommandError("You can only use --pks option with one model")

+ 1 - 1
django/core/management/commands/flush.py

@@ -93,6 +93,6 @@ Are you sure you want to do this?
         # Emit the post migrate signal. This allows individual applications to
         # respond as if the database had been migrated from scratch.
         all_models = []
-        for app_config in apps.get_app_configs(only_with_models_module=True):
+        for app_config in apps.get_app_configs():
             all_models.extend(router.get_migratable_models(app_config, database, include_auto_created=True))
         emit_post_migrate_signal(set(all_models), verbosity, interactive, database)

+ 2 - 2
django/core/management/commands/migrate.py

@@ -181,8 +181,8 @@ class Command(BaseCommand):
         all_models = [
             (app_config.label,
                 router.get_migratable_models(app_config, connection.alias, include_auto_created=True))
-            for app_config in apps.get_app_configs(only_with_models_module=True)
-            if app_config.label in app_labels
+            for app_config in apps.get_app_configs()
+            if app_config.models_module is not None and app_config.label in app_labels
         ]
 
         def model_installed(model):

+ 6 - 2
django/core/management/sql.py

@@ -207,7 +207,9 @@ def custom_sql_for_model(model, style, connection):
 
 def emit_pre_migrate_signal(create_models, verbosity, interactive, db):
     # Emit the pre_migrate signal for every application.
-    for app_config in apps.get_app_configs(only_with_models_module=True):
+    for app_config in apps.get_app_configs():
+        if app_config.models_module is None:
+            continue
         if verbosity >= 2:
             print("Running pre-migrate handlers for application %s" % app_config.label)
         models.signals.pre_migrate.send(
@@ -228,7 +230,9 @@ def emit_pre_migrate_signal(create_models, verbosity, interactive, db):
 
 def emit_post_migrate_signal(created_models, verbosity, interactive, db):
     # Emit the post_migrate signal for every application.
-    for app_config in apps.get_app_configs(only_with_models_module=True):
+    for app_config in apps.get_app_configs():
+        if app_config.models_module is None:
+            continue
         if verbosity >= 2:
             print("Running post-migrate handlers for application %s" % app_config.label)
         models.signals.post_migrate.send(

+ 3 - 3
django/db/backends/__init__.py

@@ -1271,7 +1271,7 @@ class BaseDatabaseIntrospection(object):
         from django.apps import apps
         from django.db import router
         tables = set()
-        for app_config in apps.get_app_configs(only_with_models_module=True):
+        for app_config in apps.get_app_configs():
             for model in router.get_migratable_models(app_config, self.connection.alias):
                 if not model._meta.managed:
                     continue
@@ -1292,7 +1292,7 @@ class BaseDatabaseIntrospection(object):
         from django.apps import apps
         from django.db import router
         all_models = []
-        for app_config in apps.get_app_configs(only_with_models_module=True):
+        for app_config in apps.get_app_configs():
             all_models.extend(router.get_migratable_models(app_config, self.connection.alias))
         tables = list(map(self.table_name_converter, tables))
         return set([
@@ -1307,7 +1307,7 @@ class BaseDatabaseIntrospection(object):
 
         sequence_list = []
 
-        for app_config in apps.get_app_configs(only_with_models_module=True):
+        for app_config in apps.get_app_configs():
             for model in router.get_migratable_models(app_config, self.connection.alias):
                 if not model._meta.managed:
                     continue

+ 3 - 1
django/db/migrations/loader.py

@@ -59,7 +59,9 @@ class MigrationLoader(object):
         self.disk_migrations = {}
         self.unmigrated_apps = set()
         self.migrated_apps = set()
-        for app_config in apps.get_app_configs(only_with_models_module=True):
+        for app_config in apps.get_app_configs():
+            if app_config.models_module is None:
+                continue
             # Get the migrations module directory
             module_name = self.migrations_module(app_config.label)
             was_loaded = module_name in sys.modules

+ 2 - 8
docs/ref/applications.txt

@@ -184,22 +184,16 @@ Application registry
 
     Returns ``True`` if the registry is fully populated.
 
-.. method:: apps.get_app_configs(only_with_models_module=False)
+.. method:: apps.get_app_configs()
 
     Returns an iterable of :class:`~django.apps.AppConfig` instances.
 
-    If only applications containing a models module are of interest, this method
-    can be called with ``only_with_models_module=True``.
-
-.. method:: apps.get_app_config(app_label, only_with_models_module=False)
+.. method:: apps.get_app_config(app_label)
 
     Returns an :class:`~django.apps.AppConfig` for the application with the
     given ``app_label``. Raises :exc:`~exceptions.LookupError` if no such
     application exists.
 
-    If only applications containing a models module are of interest, this method
-    can be called with ``only_with_models_module=True``.
-
 .. method:: apps.has_app(app_name)
 
     Checks whether an application with the given name exists in the registry.

+ 0 - 23
tests/apps/tests.py

@@ -26,8 +26,6 @@ SOME_INSTALLED_APPS_NAMES = [
     'django.contrib.auth',
 ] + SOME_INSTALLED_APPS[2:]
 
-SOME_INSTALLED_APPS_WTH_MODELS_NAMES = SOME_INSTALLED_APPS_NAMES[:4]
-
 
 class AppsTests(TestCase):
 
@@ -93,16 +91,6 @@ class AppsTests(TestCase):
             [app_config.name for app_config in app_configs],
             SOME_INSTALLED_APPS_NAMES)
 
-    @override_settings(INSTALLED_APPS=SOME_INSTALLED_APPS)
-    def test_get_app_configs_with_models(self):
-        """
-        Tests get_app_configs(only_with_models_module=True).
-        """
-        app_configs = apps.get_app_configs(only_with_models_module=True)
-        self.assertListEqual(
-            [app_config.name for app_config in app_configs],
-            SOME_INSTALLED_APPS_WTH_MODELS_NAMES)
-
     @override_settings(INSTALLED_APPS=SOME_INSTALLED_APPS)
     def test_get_app_config(self):
         """
@@ -117,17 +105,6 @@ class AppsTests(TestCase):
         with self.assertRaises(LookupError):
             apps.get_app_config('webdesign')
 
-    @override_settings(INSTALLED_APPS=SOME_INSTALLED_APPS)
-    def test_get_app_config_with_models(self):
-        """
-        Tests get_app_config(only_with_models_module=True).
-        """
-        app_config = apps.get_app_config('admin', only_with_models_module=True)
-        self.assertEqual(app_config.name, 'django.contrib.admin')
-
-        with self.assertRaises(LookupError):
-            apps.get_app_config('staticfiles', only_with_models_module=True)
-
     @override_settings(INSTALLED_APPS=SOME_INSTALLED_APPS)
     def test_has_app(self):
         self.assertTrue(apps.has_app('django.contrib.admin'))