Skip to content

Transparently keep reference of routers in urls generated by routers #4443

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

dimitri-justeau
Copy link

This PR slightly change the behavior of routers, to ensure that when they create a url pattern (RegexURLPattern or RegexURLResolver), it keeps a reference of the router.

The reason for this is to enhance reflexion possibilities on DRF for third-party libraries (I actually decided to suggest this because working with django-rest-framework-docs, I figured out that the support of Viewsets was complicated because the library focuses on api endpoints, and then cannot automatically discover routers - and thus methods mapping).

Important: The changes made in this PR is completely transparent for DRF users.

What I actually did: I created a subclass of django's RegexURLPattern (DrfRegexURLPattern) and a subclass of django's RegexURLResolver (DrfRegexURLResolver), both adding a router property to their super class, but behaving exactly the same. I also rewrited thedjango.conf.urls.url function to make it instantiate DrfRegexURLPattern and DrfRegexURLResolver, and replaced django's one by this one in the get_urls method of routers.

@dimitri-justeau
Copy link
Author

Can you give me a feedback on this proposition? Is it something worth considering?

@tomchristie
Copy link
Member

The short answer is: Not terribly keen on the extra complexity, no.
It'd be worth you taking a look at the schema generation and seeing if any of the problems you've cited are addressed there. If not, then what are we getting wrong in the schema generation as a result? If so, can the schema generation approach be used with the third party tools you've mentioned.

@dimitri-justeau
Copy link
Author

Ok, thank you, I'll have a look on schema generation!

@tomchristie
Copy link
Member

Will close this for now, feel free to come back to it if you've further comments and we can consider re-opening if relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants