Hi all, Thanks for the feedback. It's good to know I'm on the right track.
I've pushed another commit that addresses most of the concerns raised here. I've updated the example gist as well. I'm also curious to learn more about this "set of decorators" that can > be applied to a view by the url resolver - that feature doesn't seem to > be documented in the linked gist or in the API example. The idea is that sometimes, sets of views require a certain decorator - i.e. the whole admin uses the staff_member_required decorator (and some more stuff). This feature allows you to specify a list of decorators to apply to a whole section of your site, and prevents a lot of code duplication and the risk of missing a decorator somewhere (for example, if you add additional views to the admin, you'll have to decorate them yourself). I am not at all sure that it is a good idea to mix layers in this > suggested way, or what specific use case it improves. If the argument > that a constraint matches is an integer, why is it (in practice) useful > for the constraint to know that this integer is supposed to be the ID of > some particular model class? Why is it good for that constraint, when > reversing, to be able to take a model instance instead of an ID? To me > this sounds like the sort of implicit type coercion that makes APIs > harder to use and understand, not easier. The constraints should be very explicit about what they accept. The model_or_pk pattern is used all over Django, so I don't think it's *that* much of a problem. I see two main advantages: you can easily change the field in the url (e.g. from id to slug/uuid), and the regex pattern can be inferred from the field. The latter certainly is more friendly to beginners. But well, maybe it isn't my best idea ever. It doesn't have to be in core, it's easy enough to create in a third-party app if people want it. 1) The interplay between URL and Constraint is unclear. I moved the constraints out of the URL class, so that the Constraint knows about the URL. The URL is not strictly immutable, but I think that's difficult to avoid. Now, `Resolver.match()` is a more-or-less atomic operation that clones the old url, manipulates the clone through `Constraint.match()` and returns it. After that, the URL shouldn't change. When reversing, the URL is created from a set of constraints in one go. The URL class is something I'm still working on. I particularly like the option to build an url relative to the current request. That should gracefully handle subdomains, and it can easily be extended to handle schemes etc. It might not be needed in resolve(), though. I would like the request object available, but that could be passed as a separate argument. I don't think the pattern of cutting of the matching part should really be extended to the host, so it would only need the path and request parameters. 2) The Resolver / View API can be simplified. I was afraid that Resolver.resolve() would become too complicated, but I've managed to keep it simple. I've brought back the ResolverMatch class (which I would need for backwards compatibility anyway), and the decorator functionality has been moved over there. The resolving functionality has been moved to the Resolver class. This does feel quite a bit cleaner :) One important thing to keep in mind is reverse() performance. My (very limited) tests leave me optimistic. With some optimized constraints, reversing could be a lot faster than the current implementation, I believe. I am a little less acquainted with the internals of resolving and reversing > than the people above, and I must say that I have a hard time understanding > your new syntax when looking at the example gist. That might also have to > do with the indentation style. Would the (blog_detail, blog_index) -> > blog_resolver -> patterns be the replacement of the urlpatterns below? > Because I think the latter is a lot more readable. The example configuration is just to have a working example at hand. The final patch will have url() and include() functions, just like the current implementation. In reality you would use the urlpatterns at the bottom, not the patterns at the top. You do have a point, I've tried to clean up the example to be a bit more readable, and to work with the updated branch. Do you have an example of something that is easy to do with this API that > is hard or impossible with the current API? Domain-based resolving, for one. Another, non-obvious advantage is that a Resolver subclass can dynamically resolve or reverse patterns. A lot of apps (think flatpages, a CMS, or the sitemaps app) have a semi-static set of urls. The current approaches are limited to a catch-all pattern that further resolves in the view, or a static url for each option. The first approach has the obvious disadvantage that it can only ever be used once in a single urlconf, and the view has to solve problems that the url dispatcher should handle. With the second approach the old url dispatcher makes it virtually impossible to make runtime changes. I'm sure that's not all ;) I will probably create a separate thread to discuss the mess that is url namespaces. Marten Op zondag 17 mei 2015 16:27:32 UTC+2 schreef Tino de Bruijn: > > Hi Marten, > > I am a little less acquainted with the internals of resolving and > reversing than the people above, and I must say that I have a hard time > understanding your new syntax when looking at the example gist. That might > also have to do with the indentation style. Would the (blog_detail, > blog_index) -> blog_resolver -> patterns be the replacement of the > urlpatterns below? Because I think the latter is a lot more readable. > > Do you have an example of something that is easy to do with this API that > is hard or impossible with the current API? > > Thanks, > > > Tino > > On Sun, May 17, 2015 at 2:35 PM, Anssi Kääriäinen <akaa...@gmail.com > <javascript:>> wrote: > >> One important thing to keep in mind is reverse() performance. >> >> While it isn't important that the prototype is fast, making sure the API >> doesn't limit performance for the most important operations must be checked. >> >> At least my experience is that URL resolving performance isn't that >> important, but having fast reverse() is really important. The reason is >> that single request typically has just one resolve operation, and thus the >> amount of time used for resolving tends to end up in the noise. But large >> list/table views can have up to thousands of reverse() operations, when >> there are multiple urls per row. >> >> I believe we can have clean, fast and extensible URL resolving system, so >> I'm not too worried about this. Still, lets make sure performance won't be >> a problem. >> >> - Anssi >> >> >> On Monday, May 4, 2015, Marten Kenbeek <marte...@gmail.com <javascript:>> >> wrote: >> >>> Hi all, >>> >>> I'd like to discuss the API I'm proposing for the new url dispatcher I'm >>> working on. I'll try to explain the API and some of the rationale behind it. >>> >>> There is a working proof-of-concept at >>> https://github.com/knbk/django/tree/url_dispatcher. Currently, all the >>> names are chosen as not to conflict with the old implementation. A short >>> description can be found at >>> https://gist.github.com/knbk/96999abaab4ad4e5f8f9, which also contains >>> a link to an example url configuration. >>> >>> My main goal was to create a simple, extensible API. In the old API, >>> about 90% of the work was done in the `resolve()`, `_reverse_with_prefix()` >>> and `populate()` of the RegexURLResolver. This made for a tightly coupled >>> design that was almost impossible to extend. >>> >>> My starting point was the RegexURLResolver and RegexURLPattern. Both >>> have a regex constraint that can match the current path and extract >>> arguments. The former can then pass the remainder of the path to its list >>> of resolvers and patterns; the latter can return a ResolverMatch containing >>> the callback specified by that pattern. I've kept this distinction, with >>> the `Resolver` and `View` classes. The change of name from `Pattern` to >>> `View` is because the `View` object has a bit more logic that determines >>> how the view is called. It is a thin, callable wrapper that can optionally >>> decorate the actual view function with a set of decorators passed down from >>> parent resolvers, and when overwritten, can do some more processing before >>> or after the view function is called. >>> >>> The hard-coded dependence on a regex pattern has been abstracted to a >>> Constraint object. Each Resolver and View has a (set of) Constraint >>> object(s), that can match the current url and extract arguments. A >>> RegexPattern that simply mimics the old behaviour will be available, but >>> other implementations, such as a Constraint that matches the request's host >>> or method, are easily provided. A Constraint can also reverse a set of >>> arguments to a partial url. That means that the full set of constraints >>> used to match an url to a view, together with a suitable set of arguments, >>> can be reversed to the url itself. >>> >>> The main strength of a Constraint is that it can contain very specific >>> logic about its arguments. For example, a Constraint may know that it >>> resolves to a Model's primary key. If it then receives a Model instance of >>> that particular type, it will know how to reverse that model instance to a >>> valid string-based partial url, so that it can later be resolved to match >>> the same object. It could also e.g. infer the regex pattern from the >>> field's type. >>> >>> There's one final piece to the puzzle: the URL object. This is the state >>> of the url in the process of resolving or reversing an url. It's a two-way >>> street: when resolving, it starts out as a full path, and the Constraints >>> chip away at the path, while the set of constraints and extracted argument >>> grows. When reversing, it starts out as a set of constraints and arguments, >>> and reconstructs the partial urls from those constraints and arguments >>> until a full url path is reconstructed. It shifts some of the logic from >>> the Resolver to the URL, so that it is easier to extend the Resolver. It is >>> also a simple container that allows any Constraint access to the full >>> request. Last but not least, it allows to dynamically build an url against >>> the current request. This is useful if e.g. a constraint matches a >>> different subdomain than the current request, so that a link automatically >>> points to the right domain. >>> >>> I'm looking forwards to your feedback. >>> >>> Thanks, >>> Marten >>> >>> -- >>> You received this message because you are subscribed to the Google >>> Groups "Django developers (Contributions to Django itself)" group. >>> To unsubscribe from this group and stop receiving emails from it, send >>> an email to django-developers+unsubscr...@googlegroups.com. >>> To post to this group, send email to django-developers@googlegroups.com. >>> Visit this group at http://groups.google.com/group/django-developers. >>> To view this discussion on the web visit >>> https://groups.google.com/d/msgid/django-developers/3cb36c11-16ee-4702-92a3-f9afb177bbca%40googlegroups.com >>> >>> <https://groups.google.com/d/msgid/django-developers/3cb36c11-16ee-4702-92a3-f9afb177bbca%40googlegroups.com?utm_medium=email&utm_source=footer> >>> . >>> For more options, visit https://groups.google.com/d/optout. >>> >> -- >> You received this message because you are subscribed to the Google Groups >> "Django developers (Contributions to Django itself)" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to django-develop...@googlegroups.com <javascript:>. >> To post to this group, send email to django-d...@googlegroups.com >> <javascript:>. >> Visit this group at http://groups.google.com/group/django-developers. >> To view this discussion on the web visit >> https://groups.google.com/d/msgid/django-developers/CALMtK1FqpfeGvGC8c6P66CppXqfaS0NoKYWpjX17MsdvqWVuHQ%40mail.gmail.com >> >> <https://groups.google.com/d/msgid/django-developers/CALMtK1FqpfeGvGC8c6P66CppXqfaS0NoKYWpjX17MsdvqWVuHQ%40mail.gmail.com?utm_medium=email&utm_source=footer> >> . >> >> For more options, visit https://groups.google.com/d/optout. >> > > -- You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscr...@googlegroups.com. To post to this group, send email to django-developers@googlegroups.com. Visit this group at http://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/ccaf91b2-f0d4-4c05-a4ff-96611e3f935e%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.
