[Django] #28703: Endpoint regex pattern construction is not stable

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[Django] #28703: Endpoint regex pattern construction is not stable

Django
#28703: Endpoint regex pattern construction is not stable
-----------------------------------------+------------------------
               Reporter:  erikbryant     |          Owner:  nobody
                   Type:  Uncategorized  |         Status:  new
              Component:  contrib.admin  |        Version:  1.11
               Severity:  Normal         |       Keywords:  stable
           Triage Stage:  Unreviewed     |      Has patch:  1
    Needs documentation:  0              |    Needs tests:  0
Patch needs improvement:  0              |  Easy pickings:  0
                  UI/UX:  0              |
-----------------------------------------+------------------------
 https://github.com/django/django/blob/master/django/contrib/admin/sites.py#L277

 The regex pattern that is constructed for an endpoint is not stable. For
 instance, in my project I have seen it construct each of the following:

 ^admin/^(?P<app_label>social_django|authtoken|queues|jobs|files|users|clusters|appps|auth|vdcs)/$
 ^admin/^(?P<app_label>social_django|auth|users|vdcs|appps|jobs|queues|authtoken|clusters|files)/$
 ^admin/^(?P<app_label>authtoken|appps|social_django|clusters|users|queues|jobs|vdcs|auth|files)/$
 ^admin/^(?P<app_label>authtoken|appps|social_django|clusters|users|queues|vdcs|jobs|auth|files)/$

 This precludes the use of patterns in unit tests.

 https://github.com/django/django/pull/9230

--
Ticket URL: <https://code.djangoproject.com/ticket/28703>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/053.a05cd7d45de5382ddd023696d1574bcd%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28703: URL regex for admin's app_index view isn't constructed deterministically (was: Endpoint regex pattern construction is not stable)

Django
#28703: URL regex for admin's app_index view isn't constructed deterministically
-------------------------------------+-------------------------------------
     Reporter:  erikbryant           |                    Owner:  nobody
         Type:                       |                   Status:  new
  Cleanup/optimization               |
    Component:  contrib.admin        |                  Version:  1.11
     Severity:  Normal               |               Resolution:
     Keywords:  stable               |             Triage Stage:
                                     |  Unreviewed
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Tim Graham):

 * type:  Uncategorized => Cleanup/optimization


Old description:

> https://github.com/django/django/blob/master/django/contrib/admin/sites.py#L277
>
> The regex pattern that is constructed for an endpoint is not stable. For
> instance, in my project I have seen it construct each of the following:
>
> ^admin/^(?P<app_label>social_django|authtoken|queues|jobs|files|users|clusters|appps|auth|vdcs)/$
> ^admin/^(?P<app_label>social_django|auth|users|vdcs|appps|jobs|queues|authtoken|clusters|files)/$
> ^admin/^(?P<app_label>authtoken|appps|social_django|clusters|users|queues|jobs|vdcs|auth|files)/$
> ^admin/^(?P<app_label>authtoken|appps|social_django|clusters|users|queues|vdcs|jobs|auth|files)/$
>
> This precludes the use of patterns in unit tests.
>
> https://github.com/django/django/pull/9230
New description:

 https://github.com/django/django/blob/master/django/contrib/admin/sites.py#L277

 The regex pattern that is constructed for an endpoint is not stable. For
 instance, in my project I have seen it construct each of the following:
 {{{
 ^admin/^(?P<app_label>social_django|authtoken|queues|jobs|files|users|clusters|appps|auth|vdcs)/$
 ^admin/^(?P<app_label>social_django|auth|users|vdcs|appps|jobs|queues|authtoken|clusters|files)/$
 ^admin/^(?P<app_label>authtoken|appps|social_django|clusters|users|queues|jobs|vdcs|auth|files)/$
 ^admin/^(?P<app_label>authtoken|appps|social_django|clusters|users|queues|vdcs|jobs|auth|files)/$
 }}}
 This precludes the use of patterns in unit tests.

 https://github.com/django/django/pull/9230

--

Comment:

 It seems to me that such a URL introspection test in your project is
 testing an internal implementation in Django that could be subject to
 change in the future. Can you give an idea of what regressions you're
 looking to test for? I wonder if a test that inspects `_registry` (or
 perhaps better yet, uses the `site.is_registered()` method) would be more
 appropriate.

--
Ticket URL: <https://code.djangoproject.com/ticket/28703#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/068.37362a2719ed87044adc53562cd6dcc7%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28703: URL regex for admin's app_index view isn't constructed deterministically

Django
In reply to this post by Django
#28703: URL regex for admin's app_index view isn't constructed deterministically
-------------------------------------+-------------------------------------
     Reporter:  erikbryant           |                    Owner:  nobody
         Type:                       |                   Status:  new
  Cleanup/optimization               |
    Component:  contrib.admin        |                  Version:  1.11
     Severity:  Normal               |               Resolution:
     Keywords:  stable               |             Triage Stage:
                                     |  Unreviewed
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by erikbryant):

 I was asked to create a mechanism that would tell us if any new endpoints
 were added or if any existing endpoints were deleted. In either case we
 would need to update the web UI that uses our endpoints.

 My less-than-ideal solution was to create a snapshot of our existing
 endpoints in a file. I wrote a test that compares the current set of
 endpoints with the snapshot. If an endpoint is added/deleted the test will
 fail. The expectation is that the developer who is making the change would
 see the failure and create a ticket to have the web UI updated. I'd be
 happy to know how other teams deal with this. I'm sure there is a better
 way.

--
Ticket URL: <https://code.djangoproject.com/ticket/28703#comment:2>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/068.632f1a29bf0a7ff4456feb677e55bf5c%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28703: URL regex for admin's app_index view isn't constructed deterministically

Django
In reply to this post by Django
#28703: URL regex for admin's app_index view isn't constructed deterministically
-------------------------------------+-------------------------------------
     Reporter:  Erik Bryant          |                    Owner:  nobody
         Type:                       |                   Status:  new
  Cleanup/optimization               |
    Component:  contrib.admin        |                  Version:  1.11
     Severity:  Normal               |               Resolution:
     Keywords:  stable               |             Triage Stage:
                                     |  Unreviewed
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Tim Graham):

 I don't entirely understand your setup but I wonder if you're concerned
 with changes to admin URLs and this URL in particular? Admin URLs may
 change when upgrading Django when new features are added or removed. For
 example, Django 2.0 adds an "autocomplete view". I wonder if your
 mechanism could ignore `admin/` URLs entirely?

--
Ticket URL: <https://code.djangoproject.com/ticket/28703#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/068.cfa0359d5de80e6f058275f190c07ff5%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28703: URL regex for admin's app_index view isn't constructed deterministically

Django
In reply to this post by Django
#28703: URL regex for admin's app_index view isn't constructed deterministically
-------------------------------------+-------------------------------------
     Reporter:  Erik Bryant          |                    Owner:  nobody
         Type:                       |                   Status:  new
  Cleanup/optimization               |
    Component:  contrib.admin        |                  Version:  1.11
     Severity:  Normal               |               Resolution:
     Keywords:  stable               |             Triage Stage:
                                     |  Unreviewed
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Erik Bryant):

 That's a good point. We don't plan to test the admin/ URLs. I'll remove
 that one from the list we scrape.

 As for this ticket, should we go ahead with the fix?

--
Ticket URL: <https://code.djangoproject.com/ticket/28703#comment:4>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/068.00f58656f49fe9055c279ad93897b788%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28703: URL regex for admin's app_index view isn't constructed deterministically

Django
In reply to this post by Django
#28703: URL regex for admin's app_index view isn't constructed deterministically
-------------------------------------+-------------------------------------
     Reporter:  Erik Bryant          |                    Owner:  nobody
         Type:                       |                   Status:  new
  Cleanup/optimization               |
    Component:  contrib.admin        |                  Version:  1.11
     Severity:  Normal               |               Resolution:
     Keywords:  stable               |             Triage Stage:
                                     |  Unreviewed
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Tim Graham):

 I'm not sure if there's a compelling reason to make the proposed change
 but I guess the overhead isn't too large.

--
Ticket URL: <https://code.djangoproject.com/ticket/28703#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/068.c4d850141a914358f545f32756bef3de%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28703: URL regex for admin's app_index view isn't constructed deterministically

Django
In reply to this post by Django
#28703: URL regex for admin's app_index view isn't constructed deterministically
-------------------------------------+-------------------------------------
     Reporter:  Erik Bryant          |                    Owner:  nobody
         Type:                       |                   Status:  new
  Cleanup/optimization               |
    Component:  contrib.admin        |                  Version:  1.11
     Severity:  Normal               |               Resolution:
     Keywords:  stable               |             Triage Stage:
                                     |  Unreviewed
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Erik Bryant):

 I've been thinking of changing this from O(N-squared) to O(N) by using a
 set instead of a list. It would make the code just a little simpler. How
 about I do that to help offset the extra sorted() call?

 https://github.com/erikbryant/django/blob/master/django/contrib/admin/sites.py#L271

--
Ticket URL: <https://code.djangoproject.com/ticket/28703#comment:6>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/068.5d861a7a5f7ef698aae900a756bcd64c%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28703: URL regex for admin's app_index view isn't constructed deterministically

Django
In reply to this post by Django
#28703: URL regex for admin's app_index view isn't constructed deterministically
--------------------------------------+------------------------------------
     Reporter:  Erik Bryant           |                    Owner:  nobody
         Type:  Cleanup/optimization  |                   Status:  new
    Component:  contrib.admin         |                  Version:  1.11
     Severity:  Normal                |               Resolution:
     Keywords:                        |             Triage Stage:  Accepted
    Has patch:  1                     |      Needs documentation:  0
  Needs tests:  0                     |  Patch needs improvement:  0
Easy pickings:  0                     |                    UI/UX:  0
--------------------------------------+------------------------------------
Changes (by Tim Graham):

 * keywords:  stable =>
 * stage:  Unreviewed => Accepted


Comment:

 I think that change could be made separately without a ticket.

--
Ticket URL: <https://code.djangoproject.com/ticket/28703#comment:7>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/068.7fe7ba9fb5afaf1317d70e70e4e66452%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.