[Django] #27262: Delegate URL resolver checks to URL classes
Django
------------------------------------+--------------------
Reporter: Sjoerd Job Postmus | Owner: nobody
Type: Uncategorized | Status: new
Component: Core (URLs) | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
------------------------------------+--------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/27262>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Sjoerd Job Postmus | Owner: nobody
Type: Uncategorized | Status: new
Component: Core (URLs) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
------------------------------------+------------------------------------
Changes (by Alasdair Nicol):
* needs_better_patch: => 0
* stage: Unreviewed => Accepted
* easy: 0 => 1
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/27262#comment:1>
Django
Reporter: Sjoerd Job Postmus | Owner: nobody
Component: Core (URLs) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Tim Graham):
* type: Uncategorized => Cleanup/optimization
--
Ticket URL: <https://code.djangoproject.com/ticket/27262#comment:2>
Django
Reporter: Sjoerd Job Postmus | Owner: nobody
Component: Core (URLs) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by 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 (either `RegexURLResolver` or `RegexURLPattern`)
and called directly by `check()`
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/27262#comment:3>
Django
Reporter: Sjoerd Job Postmus | Owner: nobody
Component: Core (URLs) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by Tim Graham):
No one is assigned the ticket so you're welcome to claim it.
--
Ticket URL: <https://code.djangoproject.com/ticket/27262#comment:4>
Django
Reporter: Sjoerd Job Postmus | Owner: Lucas
Type: | Lois
Cleanup/optimization | Status: assigned
Component: Core (URLs) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Lucas Lois):
* owner: nobody => Lucas Lois
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/27262#comment:5>
Django
Reporter: Sjoerd Job Postmus | Owner: Lucas
Type: | Lois
Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Sjoerd Job Postmus):
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,
--
Ticket URL: <https://code.djangoproject.com/ticket/27262#comment:6>
Django
Reporter: Sjoerd Job Postmus | Owner: Lucas
Type: | Lois
Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Lucas Lois):
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
--
Ticket URL: <https://code.djangoproject.com/ticket/27262#comment:7>
Django
Reporter: Sjoerd Job Postmus | Owner: Lucas
Type: | Lois
Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
* stage: Accepted => Ready for checkin
Comment:
[https://github.com/django/django/pull/7332 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/27262#comment:8>
Django
Reporter: Sjoerd Job Postmus | Owner: Lucas
Type: | Lois
Component: Core (URLs) | Version: master
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"e7fa89fb58cc7ba468e0167f506dc4fce7ec532a" e7fa89fb]:
{{{
#!CommitTicketReference repository=""
revision="e7fa89fb58cc7ba468e0167f506dc4fce7ec532a"
Fixed #27262 -- Moved URL checks to resolver and pattern classes.
Thanks Sjoerd Job Postmus for the report and review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27262#comment:9>