Corrected docs on router include with namespaces. #5843

carltongibson · 2018-02-20T19:52:54Z

For Django 2.0 you MUST provide app_name when using namespace.

For Django 2.0 you MUST provide `app_name` when using `namespace`. Closes encode#5659 Correct namespace reference Updated example without changing old text.

tomchristie · 2018-02-22T12:18:56Z

docs/api-guide/routers.md

+Router URL patterns can also be included using _application_ and _instance_ namespaces.
+
+To use an _application namespace_ pass a 2-tuple containing the `router.urls` and the app name.
+To use an _instance namespace_ pass the optional `namespace` parameter.


Can we reference anywhere in the Django docs here? I wouldn't know the difference between the two off the top of my head.

I guess ideally we'd just say "Do X", and have "If you need to do Y then ..." slightly later in the docs, rather than presenting the developer with a choice that they need to make here.

The [URL namespaces docs] reference below is the canonical source. It's basically the only place it's ever discussed.

The other place we could link to is the include reference https://docs.djangoproject.com/en/2.0/ref/urls/#include

The way I thought about phrasing this is:

Try to include via an app-level urls module, setting the app_name variable there. Basically never use the namespace parameter, you basically don't want it.

If you MUST include the router urls directly (and need namespaces) then provide (router.urls, app_name) — that's what you want. (If you really need namespace then go-ahead, but you still have to pass the 2-tuple first.)

But by the time I'd drafted than I basically need to rewrite the Django docs (linked) so people should really read those.

The >80% case is to just pass the 2-tuple and ignore the namespace parameter. Let me re-phrase around that...

carltongibson

Hey @tomchristie.

I adjusted the phrasing. Have a look.

(It's tricky because the whole application vs instance thing isn't clearly represented by the API...)

carltongibson · 2018-02-22T12:44:05Z

docs/api-guide/routers.md


    urlpatterns = [
        url(r'^forgot-password/$', ForgotPasswordFormView.as_view()),
-        url(r'^api/', include(router.urls, namespace='api')),
+        url(r'^api/', include((router.urls, 'app_name')),


This is the >80% usage for namespaces. (We want to namespace our URL names to a single application.)

The include(( ... ) looks wrong to me?

I'm confused by the app_name argument, does it have to match the app name, or is it a general purpose namespace?

The parenthesis is missing. I'll fix that.

SO... 😃

What ultimately gets passed included is a 3-tuple:

(A_LIST_OF_PATTERNS, AN_APPLICATION_NAMESPACE, AN_INSTANCE_NAMESPACE)

In the example here we pass the first two of those, as a 2-tuple.

When you include a module the list of patterns comes from urlpatterns, and the application namespace comes from app_name.

I've just used app_name here as any old string. It's usually, 'admin', or 'polls' or 'rest_framework'.

In this example the instance namespace ends up being the same as the application namespace, because we didn't provide one. (This means that it'll be None if the application namespace is, which is just the global namespace we all start out with.)

But we could do provide, say, namespace='v1' and such.

Grrr. I'm still not happy here.

carltongibson · 2018-02-22T12:44:35Z

docs/api-guide/routers.md

-If using namespacing with hyperlinked serializers you'll also need to ensure that any `view_name` parameters on the serializers correctly reflect the namespace. In the example above you'd need to include a parameter such as `view_name='api:user-detail'` for serializer fields hyperlinked to the user detail view.
+(`include` also allows an optional `namespace` parameter to also create an _instance namespace_.
+This defaults to `None` and will be set to `app_name` if not provided.
+See Django's [URL namespaces docs][url-namespace-docs] for more details.)


Do we even need to say this? Not sure...

Looks better!

and will be set to app_name ...

Grrrr. "...the same as the application namespace..." or such.

Don't merge this yet. I'll have a go over the weekend.

carltongibson · 2018-02-22T15:51:45Z

docs/api-guide/routers.md

@@ -72,14 +72,21 @@ Alternatively you can use Django's `include` function, like so…
        url(r'^', include(router.urls)),
    ]

-Router URL patterns can also be namespaces.
+To include the router URL patterns with an application namespace pass a
+`(router.urls, 'app_name')` 2-tuple to `include`.


I added the '' around app_name here. Hopefully that works.

The option is something like (PATTERN_LIST, APP_NAME)

carltongibson · 2018-02-22T15:52:42Z

docs/api-guide/routers.md

    ]

-If using namespacing with hyperlinked serializers you'll also need to ensure that any `view_name` parameters on the serializers correctly reflect the namespace. In the example above you'd need to include a parameter such as `view_name='api:user-detail'` for serializer fields hyperlinked to the user detail view.
+(You may additionally pass an optional `namespace` parameter to `include` to also
+create an _instance namespace_. See Django's [URL namespaces docs][url-namespace-docs] for more details.)


Maybe if I drop the whole "...if namespace=None bit... and just let people read the docs.

Yup. Although do let's reference Django's include docs here - I think that'd be helpful.

carltongibson · 2018-03-06T08:13:00Z

Hey @tomchristie. Can I ask you to pass your eyes over this again.

8f821dc Gives both app and instance namespace examples and links to both parts of the Django docs. I think it's about as clear as can be (without it becoming an essay).
e274bcd adds a little emphasis to the idea that you probably don't need namespaces, which is relevant when you're using Hyperlinked.... I keep having the same conversation around that, so I just want to draw it out. (I don't think people would be using namespaces at all unless we showed them).

* Provide both app and instance namespace examples * Emphasise non-namespaced option

carltongibson requested a review from tomchristie February 20, 2018 19:53

carltongibson added the Documentation label Feb 20, 2018

carltongibson added this to the 3.8 Release milestone Feb 20, 2018

Corrected docs on router include with namespaces.

3ba3f45

For Django 2.0 you MUST provide `app_name` when using `namespace`. Closes encode#5659 Correct namespace reference Updated example without changing old text.

carltongibson force-pushed the 38/router-include-docs branch from b5be07f to 3ba3f45 Compare February 20, 2018 20:08

tomchristie reviewed Feb 22, 2018

View reviewed changes

Adjust phrasing.

e88d46a

carltongibson commented Feb 22, 2018

View reviewed changes

carltongibson added 2 commits February 22, 2018 16:23

Add missing parenthesis

51884ab

Adjust wording

a0528fd

carltongibson commented Feb 22, 2018

View reviewed changes

carltongibson added 2 commits March 6, 2018 09:08

Provide both app and instance namespace examples

8f821dc

Emphasise non-namespaced option

e274bcd

carltongibson merged commit 0da4617 into encode:master Mar 13, 2018

carltongibson deleted the 38/router-include-docs branch March 13, 2018 14:52

carltongibson mentioned this pull request Mar 13, 2018

HyperlinkedModelSerializer fails when app_name is configured and URLs added to app. #5850

Closed

6 tasks

TheMickeyMike mentioned this pull request Mar 19, 2018

BOOK-26 | Create Comments App rogemus/BookHub#6

Merged

pchiquet pushed a commit to pchiquet/django-rest-framework that referenced this pull request Nov 17, 2020

Corrected docs on router include with namespaces. (encode#5843)

aaf55ac

* Provide both app and instance namespace examples * Emphasise non-namespaced option

Uh oh!

Corrected docs on router include with namespaces. #5843

Corrected docs on router include with namespaces. #5843

Uh oh!

Conversation

carltongibson commented Feb 20, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carltongibson Feb 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carltongibson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carltongibson commented Mar 6, 2018

Uh oh!

Uh oh!

carltongibson Feb 22, 2018 •

edited

Loading