From bfcc686d22df10c268752635947dd242317ba156 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Mon, 30 Dec 2013 23:53:54 +0100 Subject: [PATCH] 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. --- django/apps/registry.py | 35 +++++++++------------ django/contrib/contenttypes/management.py | 2 +- django/core/management/base.py | 3 -- django/core/management/commands/dumpdata.py | 4 +-- django/core/management/commands/flush.py | 2 +- django/core/management/commands/migrate.py | 4 +-- django/core/management/sql.py | 8 +++-- django/db/backends/__init__.py | 6 ++-- django/db/migrations/loader.py | 4 ++- docs/ref/applications.txt | 10 ++---- tests/apps/tests.py | 23 -------------- 11 files changed, 34 insertions(+), 67 deletions(-) diff --git a/django/apps/registry.py b/django/apps/registry.py index a6b807e608..bdd566c388 100644 --- a/django/apps/registry.py +++ b/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]) diff --git a/django/contrib/contenttypes/management.py b/django/contrib/contenttypes/management.py index 1b499d85f1..58f7da820e 100644 --- a/django/contrib/contenttypes/management.py +++ b/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) diff --git a/django/core/management/base.py b/django/core/management/base.py index a71cc78588..e55f777b75 100644 --- a/django/core/management/base.py +++ b/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: diff --git a/django/core/management/commands/dumpdata.py b/django/core/management/commands/dumpdata.py index d4d8b54af6..1242bd4e57 100644 --- a/django/core/management/commands/dumpdata.py +++ b/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") diff --git a/django/core/management/commands/flush.py b/django/core/management/commands/flush.py index 5b4733ecad..4a3f7c2d8b 100644 --- a/django/core/management/commands/flush.py +++ b/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) diff --git a/django/core/management/commands/migrate.py b/django/core/management/commands/migrate.py index 5da5e51a48..cb863509c8 100644 --- a/django/core/management/commands/migrate.py +++ b/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): diff --git a/django/core/management/sql.py b/django/core/management/sql.py index 4cced4f566..d2c7786390 100644 --- a/django/core/management/sql.py +++ b/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( diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py index 5b681756e9..00ae64e09b 100644 --- a/django/db/backends/__init__.py +++ b/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 diff --git a/django/db/migrations/loader.py b/django/db/migrations/loader.py index a3ed25793a..66d2259b79 100644 --- a/django/db/migrations/loader.py +++ b/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 diff --git a/docs/ref/applications.txt b/docs/ref/applications.txt index 5d9db49140..eadf9895ca 100644 --- a/docs/ref/applications.txt +++ b/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. diff --git a/tests/apps/tests.py b/tests/apps/tests.py index 326ddcb6ab..2a41acf2e8 100644 --- a/tests/apps/tests.py +++ b/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'))