pakal / django-compat-patcher

A system to improve compatibility between different Django versions, and make upgrading dependencies less painful.
MIT License
13 stars 2 forks source link

Specifying a namespace in include() without providing an app_name is not supported #15

Closed jayvdb closed 2 years ago

jayvdb commented 4 years ago

I am following the instructions of https://github.com/peopledoc/django-formidable using recently released 5.0.0 on a Django 3 project. It supports only Django 2.2, so this is like a new aspect for Django 3.

  File "path/to/project/urls.py", line 209, in <module>
    path('api/', include('formidable.urls', namespace='formidable')),
  File "/usr/lib/python3.8/site-packages/django_compat_patcher/fixers/django2_0.py", line 236, in include
    return original_include(arg, namespace=namespace)
  File "/usr/lib/python3.8/site-packages/django/urls/conf.py", line 38, in include
    raise ImproperlyConfigured(
django.core.exceptions.ImproperlyConfigured: Specifying a namespace in include() without providing an app_name is not supported. Set the app_name attribute in the included module, or pass a 2-tuple containing the list of patterns and app_name instead.
pakal commented 4 years ago

Strange, it was in Django Release 2.0 notes : "Support for setting a URL instance namespace without an application namespace will be removed."
How does it work in Django2.2 ? :?

It's hard to follow the changes around these urlconfs, namespaces and app_names (there have been lots of refactorings in the Django codebase on this side), I guess the simplest way to solve this without breaking future code would be to force a default "app_name" value when there is a namespace - like just "default"?

pakal commented 4 years ago

The commit where this appeared: https://github.com/django/django/commit/1e82094f1b6690018228e688303295f83e1c3d9a I have no idea of all the impacts of having a None app_name, in previous versions of Django

pakal commented 3 years ago

I tried latest master of django-formidable, it seems to work fine with DCP except some little regression in testcase records.

In your traceback it seems that it's the PROJECT which is including "formidable" without app_name - in this case the fix is on the side of the project, not of dependencies, so no need for DCP.

I don't know if django apps might themselves use include() with namespaces, if so we'll have to look how it should be solved.

jayvdb commented 3 years ago

In your traceback it seems that it's the PROJECT ...

Sorry, that was a bit unclear. Yes, the problem was in the projects urls, and can be fixed by changing how it is added the the urlconf, but that means the project urls needs both if switching DCP on and off - I do this switching on/off when updating deps and checking which apps need patches sent upstream.

Agree this is just a nice-to-have unless we find apps which cause this problem - no doubt they exist, but probably not too often.

The workaround is likely adding an app_name = 'installed_app' attribute to the installed_app.urls module.

pakal commented 3 years ago

Yes, let's see if anyone needs this fix I guess, it'll help making sure the fixer is relevantly implemented B-)

pakal commented 2 years ago

I'm closing this, since this is visibly fixable at project level B-)