Namespaces not taken into account

[henhuy]

I wanted to distill views from an app (via python manage.py distill-local), but I think distill does not take namespaces into account... I tried with following setup:

# base urls.py
urlpatterns = [
    ...
    path("", include("djagora.map.urls", namespace="map")),
]

# map/urls.py
app_name = "map"

urlpatterns = [
    ...
    distill_path(
        "some_view/",
        view=views.SomeView.as_view(),
        name="some",
    ),
]

But this lead to an error when generating uri: https://github.com/meeb/django-distill/blob/ea6132b6c21c73265c5b80389c9fb448c9eb7297/django_distill/renderer.py#L81 as view_name only contains "some", but correct reverse name would be "map:some". After setting view_name = "map:some" within debugging mode, script goes on without error (giving me some errors later, but this does not belong here). Can you confirm this error, or am I missing something?

[meeb]

Could you paste the full stack trace from your error please? This sounds like something that should be easy to work around or fix. In theory this should work fine, all distill does is pass on all the kwargs to the usual Django path() so if there is a bug here it's probably just in the way distill iterates the stored URLs to render later.

[henhuy]

Strack trace:

Loading site URLs
Traceback (most recent call last):
  File "/home/local/RL-INSTITUT/hendrik.huyskens/Dokumente/RLI/djagora/djagora/manage.py", line 30, in <module>
    execute_from_command_line(sys.argv)
  File "/home/local/RL-INSTITUT/hendrik.huyskens/miniconda3/envs/djangora/lib/python3.8/site-packages/django/core/management/__init__.py", line 401, in execute_from_command_line
    utility.execute()
  File "/home/local/RL-INSTITUT/hendrik.huyskens/miniconda3/envs/djangora/lib/python3.8/site-packages/django/core/management/__init__.py", line 395, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/home/local/RL-INSTITUT/hendrik.huyskens/miniconda3/envs/djangora/lib/python3.8/site-packages/django/core/management/base.py", line 330, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/home/local/RL-INSTITUT/hendrik.huyskens/miniconda3/envs/djangora/lib/python3.8/site-packages/django/core/management/base.py", line 371, in execute
    output = self.handle(*args, **options)
  File "/home/local/RL-INSTITUT/hendrik.huyskens/miniconda3/envs/djangora/lib/python3.8/site-packages/django_distill/management/commands/distill-local.py", line 82, in handle
    render_to_dir(output_dir, urls_to_distill, stdout)
  File "/home/local/RL-INSTITUT/hendrik.huyskens/miniconda3/envs/djangora/lib/python3.8/site-packages/django_distill/renderer.py", line 189, in render_to_dir
    for page_uri, file_name, http_response in renderer.render():
  File "/home/local/RL-INSTITUT/hendrik.huyskens/miniconda3/envs/djangora/lib/python3.8/site-packages/django_distill/renderer.py", line 81, in render
    uri = self.generate_uri(view_name, param_set)
  File "/home/local/RL-INSTITUT/hendrik.huyskens/miniconda3/envs/djangora/lib/python3.8/site-packages/django_distill/renderer.py", line 113, in generate_uri
    uri = reverse(view_name, kwargs=param_set)
  File "/home/local/RL-INSTITUT/hendrik.huyskens/miniconda3/envs/djangora/lib/python3.8/site-packages/django/urls/base.py", line 87, in reverse
    return iri_to_uri(resolver._reverse_with_prefix(view, prefix, *args, **kwargs))
  File "/home/local/RL-INSTITUT/hendrik.huyskens/miniconda3/envs/djangora/lib/python3.8/site-packages/django/urls/resolvers.py", line 685, in _reverse_with_prefix
    raise NoReverseMatch(msg)
django.urls.exceptions.NoReverseMatch: Reverse for 'distilled_forest' not found. 'distilled_forest' is not a valid view function or pattern name.

[meeb]

I'll write a test case to check for namespaced inclusion and push any fixes into the next distill release when I get a moment.

[meeb]

I've had a look into this and it could be pretty complicated to properly support namespaces. Internally Django stores URLs as a tree. The issue is the wrapper functions like distill_path(...) have no way to know what their current namespace is as that's defined by the include(...) provided namespace name in the parent app urls.py. To properly support namespaces and nested namespaces you can't just wrap include(...) with a distill_include(...) as it would potentially require other apps out of the developers control to use distill_* hooks. It may be possible to try and probably monkeypatch into Django internals but that is going to be messy and prone to breaking. In the immediate short term the advice is don't use distill in sub-application namespaces for now until there's an obvious way around it without sacrificing the simplicity of the interface distill currently has. I'll pop it on the roadmap to dig into the Django internals to see if this can be resolved in a future release without breaking backwards compatibility.

[henhuy]

Thank you anyway for going down the django rabbit hole!

[meeb]

In the just pushed commit adds partial namespace support including nested namespaces. The only situation where namespaces are not supported is include()ing the same set of URLs more than once on the same project, which will raise a polite and detailed error. So this is fine:

urlpatterns = [
    path('namespace/',
        include('someapp.urls', namespace='namespace')),
    path('namespace/',
        include('anotherapp.urls', namespace='anothernamespace')),
]

But this will throw an error:

urlpatterns = [
    path('namespace/',
        include('someapp.urls', namespace='namespace')),
    path('namespace2/
        include('someapp.urls', namespace='namespace2')),
]

As in, including someapp.urls more than once in the same project. Different URLs are fine. This is because the only sane way to map URLs to their namespace is to use the URL function itself (which matches the id(urlfunc) in most Python interpreters). The second example would reference the same URL more than once, making it impossible to work out which namespace it belongs to. Hopefully this partial implementation is sufficient for all but the most unusual deployments.

[henhuy]

Very cool. Thanks! (And I can't see any reason to put in URL paths twice - so I think it's fine.)