Opened 8 years ago
Closed 8 years ago
#27262 closed Cleanup/optimization (fixed)
Delegate URL resolver checks to URL classes
Reported by: | Sjoerd Job Postmus | Owned by: | Lucas Lois |
---|---|---|---|
Component: | Core (URLs) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
As discussed in http://sjoerdjob.com/post/is-djangos-url-routing-tightly-coupled/ and https://groups.google.com/d/msg/django-developers/u6sQax3sjO4/t6FfSex1AwAJ, the URL checking logic is coupled to the current implementation.
It would be better to implement a .check()
method on the URL resolver classes themselves, and move the logic to the relevant classes.
Change History (9)
comment:1 by , 8 years ago
Easy pickings: | set |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 8 years ago
Type: | Uncategorized → Cleanup/optimization |
---|
comment:3 by , 8 years ago
comment:5 by , 8 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:6 by , 8 years ago
Hi Lucas Lois,
Great to hear you are interested in working on this Django ticket.
I have already started the implementation of this a while ago. I pushed my changes today, to my fork on github (after your post).
Seeing as you are also interesting on working on this ticket, I would be happy to not create a pull request and give you a chance to also implement this. I'd be happy to also review of your changes.
If instead you'd prefer to not duplicate the effort and choose another ticket, go ahead and I'll create a pull-request of my changes.
Your implementation plan makes sense to me, although you should be aware of the recursive nature of URL resolvers.
Kind regards,
comment:7 by , 8 years ago
Hi Sjoerd,
Thanks for giving me the opportunity! It's a shame I only read this now, as I already wrote my implementation while offline. I guess now that we have duplicated out efforts I might as well do my PR. I'm sure I'll get some great advice from you all
Thanks for everything (and I'm sorry we had to write this twice),
Lucas Lois
I would like to implement this, if possible.
What I thought about, and gathered form the linked post by Sjoerd Job, was transforming the check_resolver function into, basically, a simple loop callign
pattern.check()
and adding together all warnings.If I understand correctly,
check_*(pattern)
should become members of their respective classes (eitherRegexURLResolver
orRegexURLPattern
) and called directly bycheck()
Could I be assigned this task? Is there anything I haven't thought about? Keep in mind I haven't commenced the implementation yet, just thought about it.