Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#27362 closed Cleanup/optimization (needsinfo)

Omitting default_app_config in __init__.py happens too easily.

Reported by: Thomas Güttler Owned by: nobody
Component: Core (Other) Version: 1.10
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Up to now it is likely that the programmer forgets to add "default_app_config = 'myapp.apps.AppConfig" in __init__.py.

It took too much time to debug why the signal handled of a reusable app was not called on one system (INSTALLED_APPS=[... 'myapp'])
and called on another system (INSTALLED_APPS=[..., 'myapp.apps.AppConfig']).

I have no clue how to solve this at the moment.

But I hope you can understand that "pain".

What can be done to assist the programmer?

Related docs: https://docs.djangoproject.com/en/1.10/ref/applications/#configuring-applications

Change History (6)

comment:1 by Aymeric Augustin, 8 years ago

Yes, I can see the problem.

That's one of the reasons why I'm against default_app_config. It's only there for backwards compatibility and should only be used by pluggable apps that predate Django 1.7, need a custom app config and don't want to ask their users to update their INSTALLED_APPS setting.

Unfortunately, even if default_app_config was removed, we'd still be stuck with two kinds of apps: those that provide a custom AppConfig and those that don't. This results in a mixed style in INSTALLED_APPS, and then it's easy to reference an app by its package name and load it with the base AppConfig class instead of its custom MyAppConfig class.

Like you I'm not really coming up with great ideas to improve the situation...

comment:2 by Tim Graham, 8 years ago

Component: UncategorizedCore (Other)
Resolution: needsinfo
Status: newclosed
Summary: Omitting default_app_config in __init__.py happens to easily.Omitting default_app_config in __init__.py happens too easily.
Type: UncategorizedCleanup/optimization

There was #25356 in which it was suggested that default_app_config is discouraged and shouldn't be in the default app template.

Closing as needsinfo absent a way to move forward with the ticket. Feel free to reopen if anyone thinks of actionable items.

comment:3 by Thomas Güttler, 8 years ago

What kind of info is needed?

comment:4 by Tim Graham, 8 years ago

A proposed solution.

comment:5 by Aymeric Augustin, 8 years ago

Historically this ticket would have been kept open in the "design decision needed" stage. We started closing tickets with no clear way forwards a couple years ago to keep open tickets actionable.

I believe that the documentation is clear is it stands and (unfortunately) not a good solution here because a programmer debugging such a problem is unlikely to be perusing the documentation of apps.

Perhaps we could add a check to raise a warning if there's an AppConfig subclass defined in an apps submodule -- the conventional location? That seems quite prone to false positives...

Other arguments or ideas welcome...

comment:6 by Thomas Güttler, 7 years ago

We use this test to unsure that the same error does happen again:

    def test_app_config_registered(self):
        for app_config in apps.get_app_configs():
            if not app_config.__module__=='django.apps.config':
                # This app has an own AppConfig class registered: ok
                continue
            apps_module_path='%s.apps' % app_config.name
            try:
                import_module(apps_module_path)
            except ImportError:
                # No module called "apps": ok
                continue
            raise AssertionError('%s exists, but the registry (apps.get_app_configs()) contains the base class. Please use dotted python path to AppConfig in INSTALLED_APPS or use default_app_config in __init__.py' %
                             (apps_module_path))

I guess this can't be integrated into django.

Note: See TracTickets for help on using tickets.
Back to Top