-
-
'clear_url_caches' isn't being used here. Did you use it here initially and then factor out to DynamicURLResolver?
-
-
I'm not sure if this is a typo, or purposeful, but shouldn't we have: super(DynamicURLResolver, self).__init__(regex=regex, If it is purposeful, and we're just discarding the passed in regex, maybe we should document that, or not specify it as a kwarg.
-
I'm a little confused about this whole urlconf_name and urlconf_module business. Have you repurposed urlconf_module to be what RegexURLResolver's url_patterns usually is? If I'm correct, what was the purpose of this? I feel like we're jumping through a lot of hoops just to have url_patterns return a list. Wouldn't it be more readable if we just did away with the concept of urlconf_* for this class, and had url_patterns be the list?
-
Looking at RegexURLResolver's url_patterns property, it appears to return: getattr(self.urlconf_module, "urlpatterns", self.urlconf_module) Is there a reason why we're no longer trying to return self.urlconf_module.urlpatterns, and just skipping straight to self.urlconf_module? Is it because we don't care about supporting urls.py files like regular django apps, with this? This is related to my other comment about urlconf_name
Introduce DynamicURLResolver, and use it for extensions.
Review Request #3577 — Created Nov. 28, 2012 and submitted
Introduce DynamicURLResolver, and use it for extensions. Getting dynamic URL patterns working in Django is tricky, since Django assumes URL patterns will be set once and will never change. DynamicURLResolver is the answer to that. It takes the place of a url() in a urlpatterns list, and provides add_patterns() and remove_patterns() functions that callers can use to modify the URL patterns list. To do this, it manipulates its own list of URL patterns, and the caching of every URL resolver between the root resolver and itself. Whenever the list is manipulated, all resolvers in that chain have their caches emptied, causing the next lookup to traverse up to the DynamicURLResolver. This means there's a little bit of overhead every time the list is manipulated, but it's not terrible, and won't affect resolvers outside that chain. This is being used for URLHooks, WebAPIResources on extensions, and admin URLs for extensions.
Tested a repro case we found where API resources for models in extensions weren't able to reverse a URL from inside that extension's API tree properly. This now works fine. Added unit tests, which pass (and failed before). More work will need to be done to ensure URLHooks work properly in all cases, and unit tests should be written there as well.
| Description | From | Last Updated |
|---|---|---|
|
'clear_url_caches' isn't being used here. Did you use it here initially and then factor out to DynamicURLResolver? |
SM smacleod | |
|
I'm not sure if this is a typo, or purposeful, but shouldn't we have: super(DynamicURLResolver, self).__init__(regex=regex, If it is purposeful, … |
SM smacleod | |
|
I'm a little confused about this whole urlconf_name and urlconf_module business. Have you repurposed urlconf_module to be what RegexURLResolver's url_patterns … |
SM smacleod |
SM
- Change Summary:
-
* Fixed the regex being passed to the parent's constructor. * Removed an unused import.
- Change Summary:
-
* Made admin URLs work in extensions. * Introduced ExtensionManager.dynamic_urls, which can be used for any hooks or extensions that need to inject URLs sanely. * Consumers of ExtensionManager now need to include ExtensionManager.get_url_patterns. * URLHook is now simpler, using dynamic_urls.
- Description:
-
Introduce DynamicURLResolver, and use it for extensions.
Getting dynamic URL patterns working in Django is tricky, since Django
assumes URL patterns will be set once and will never change. DynamicURLResolver is the answer to that. It takes the place of a url()
in a urlpatterns list, and provides add_patterns() and remove_patterns() functions that callers can use to modify the URL patterns list. To do this, it manipulates its own list of URL patterns, and the caching
of every URL resolver between the root resolver and itself. Whenever the list is manipulated, all resolvers in that chain have their caches emptied, causing the next lookup to traverse up to the DynamicURLResolver. This means there's a little bit of overhead every time the list is manipulated, but it's not terrible, and won't affect resolvers outside that chain. ~ This is being used for URLHooks and for WebAPIResources on extensions.
~ We may want to use it for admin URLs as well, but this needs ~ This is being used for URLHooks, WebAPIResources on extensions, and
~ admin URLs for extensions. - investigation.