Admindocs' View index assumes settings.ROOT_URLCONF is an import string

[Keryn Knight]

Currently, the code looks like:

urlconf = import_module(settings.ROOT_URLCONF)
view_functions = extract_views_from_urlpatterns(urlconf.urlpatterns)

Buuuuuuuut, it's possible to get Django to otherwise run with, for example, something like:

def urlpatterns():
    from django.urls import path
    from django.contrib import admin
    return (
        path("admin/", admin.site.urls),
    )

ROOT_URLCONF = SimpleLazyObject(urlpatterns)

which will happily pass through the checks framework, and runserver (and gunicorn), and let you use Django as you otherwise would... except for navigating to django.contrib.admindocs.views.ViewIndexView (for example, routed at /admin/doc/views/) where you'll instead encounter an exception like:

File "/path/to/python3.7/site-packages/django/contrib/admindocs/views.py" in dispatch
  46.         return super().dispatch(request, *args, **kwargs)

File "/path/to/python3.7/site-packages/django/views/generic/base.py" in dispatch
  97.         return handler(request, *args, **kwargs)

File "/path/to/python3.7/site-packages/django/views/generic/base.py" in get
  158.         context = self.get_context_data(**kwargs)

File "/path/to/python3.7/site-packages/django/contrib/admindocs/views.py" in get_context_data
  139.         urlconf = import_module(settings.ROOT_URLCONF)

File "/path/to/python3.7/importlib/__init__.py" in import_module
  118.     if name.startswith('.'):

File "/path/to/python3.7/site-packages/django/utils/functional.py" in inner
  257.         return func(self._wrapped, *args)


Exception Type: AttributeError at /admin/doc/views/
Exception Value: 'tuple' object has no attribute 'startswith'

[Carlton Gibson]

Hi.

Docs have it thus:

ROOT_URLCONF: A string representing the full Python import path to your root URLconf, for example "mydjangoapps.urls". ...

So I'm kind of inclined towards wontfix. ViewIndexView is making a reasonable assumption.

Buuuuuuuut, it's possible to get Django to otherwise run with, for example, something like...

Can I ask what the use-case here is? (Update: like, I've done similar defining ROOT_URLCONF = __name__, which then pulls urlpatterns into the resolver... — just trying to grasp if we need to allow ROOT_URLCONF as a callable, rather than an import path.)

(Also: How does it go with tests and override_settings and such? Is this literally the only place it blows up?)

[Keryn Knight]

Docs have it thus [...]
So I'm kind of inclined towards wontfix. ViewIndexView is making a reasonable assumption.

It is making a generally reasonable assumption, but it's an assumption which is sort of wrong.
The documentation is also misleading anyway, because it says Not defined is the default which is ... true (being that it's not in global_settings), but at least on 2.2, not having a ROOT_URLCONF is your project settings is an AttributeError:

/path/to/python3.7/site-packages/django/core/handlers/base.py", line 74, in get_response
    set_urlconf(settings.ROOT_URLCONF)
AttributeError: 'Settings' object has no attribute 'ROOT_URLCONF'

So ROOT_URLCONF must be set, fine. It is on any given startproject driven package anyway. Let's set it to something which might be considered "not defined" and do ROOT_URLCONF = None because we know ​how Django processes a request must allow for some variation upon it... and use a middleware instead:

class URLPatternsMW(MiddlewareMixin):
    def process_request(self, request):
        request.urlconf = urlpatterns()

That works fine, and lo we can navigate to the admin and all around it as you might expect (and also most of admindocs, because in my original ticket I stupidly omitted that route from the urlpatterns() definition). But if we try to go to the same URL as before (/admin/doc/views/), we get the same principle error, but on a different type:

'NoneType' object has no attribute 'startswith'

Perhaps we should've set ROOT_URLCONF = "" and tried that instead:

Exception Type: ValueError at /admin/doc/views/
Exception Value: Empty module name

It also cannot be a "myproject.urls" where there's no urlpatterns attribute, but it can if urlpatterns = [] is within that module - that's at least dealt with by the checks framework. So I guess that's the actual intended way of having no root urlconf to speak of... Ten years in, and I didn't actually know that :)
ROOT_URLCONF can be () and it's also seemingly fine (because set_urlconf does the weaker falsy test), again except for this import expectation. Mostly because these things are hashable.

Can I ask what the use-case here is? (Also: How does it go with tests and override_settings and such? Is this literally the only place it blows up?)

It's actually specifically within tests where I found it (or remembered it, I do think I encountered it before but just moved on with life), by doing:

if __name__ == "__main__":
    def urlpatterns(...):
        return ()
    settings.configure(ROOT_URLCONF=SimpleLazyObject(urlpatterns), ...)
    django.setup()
    run the tests etc.

I happen to "know" (hah!) using the deferred object mostly seems to work from other weird hacks and things I've played around with over the years (I think for example I may've been doing something similar in #26287), but I cannot say with certainty that it's the only place it poses an error (and certainly a bunch of downstream packages would make the same assumption).
I can say that it's the only place in Django where I know the error can happen, being that it's the only place which directly tries to import the value of ROOT_URLCONF, where most everything else handles it further down the resolver stack.

A cursory glancing guess (as is always my way, I'm afraid) at a fix is something more like:

        urlconf = get_resolver(settings.ROOT_URLCONF)
        try:
            view_functions = extract_views_from_urlpatterns(urlconf.url_patterns)
        except ImproperlyConfigured:
            view_functions = []

which is more like what everything else looks to do (using the threadlocal resolver). I don't offer that up as the full solution, but a possible direction in which a correction might be found.

[Carlton Gibson]

Thanks for the update Keryn. Let's accept this, since it seems we could investigate bullet proofing the admindoc view there.

Nice example.

[Qihao(Jim) Xie]

Hey! I'm a first time contributor and I would like to put in a patch to fix this issue. I'll start by understanding the source code. Any help/guidance on which module to look for in the source code would be much appreciated!

[Carlton Gibson]

Hi Qihao, welcome

​The code in question is in here. Once you're up and running, you can post to the ​forum if you need help.

[Qihao(Jim) Xie]

Replying to Carlton Gibson:

Hi Qihao, welcome

​The code in question is in here. Once you're up and running, you can post to the ​forum if you need help.

Hey Carlton, thank you for pointing me to the code in question! Since I'm not familiar with Django's codebase yet, my plan is to try to reproduce the issue suggested by Keryn in the original ticket and possibly write a testcase for it by the end of this week. From there I'll spend a week or two working on the patch and the documentation for the patch. I'll keep commenting this ticket as I make progress.

[Qihao(Jim) Xie]

Here is what I have for now: PR​https://github.com/django/django/pull/13748. Happy to see any comments!

[Carlton Gibson <carlton@…>]

In ce60d289:

Fixed #31527 -- Allowed admindocs index to handle non-string URLconfs.