Opened 4 years ago

Last modified 2 years ago

#32565 new Cleanup/optimization

Extract URL resolver view strings mapping to admindocs

Reported by: Adam Johnson Owned by: Alokik Roy
Component: Core (URLs) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

From discussion with Nick Pope: ​https://github.com/django/django/pull/14138#discussion_r595911054

URLResolver._is_callback() , URLResolver._callback_strs, and URLPattern.lookup_str all exist only to power django.contrib.admindocs functionality, and are all private attributes. The functionality they provide could be extracted into admindocs to avoid the overhead of storing extra strings per URL in projects not using admindocs (in my experience, that is most projects).

Change History (16)

comment:1 by Carlton Gibson, 4 years ago

Hmmm 🤔 — internally these are only used by admindocs. I wonder if they're not being used by tooling somewhere… (​asked ref PyCharm for an initial data point.)

comment:2 by Adam Johnson, 4 years ago

I had the thought that, at the very least, we could make generation lazy.

comment:3 by Carlton Gibson, 4 years ago

Triage Stage: Unreviewed → Accepted

Negative from PyCharm: they are not using these attributes. And if they’re not I’m inclined to punt on no-one really is.

comment:4 by Alokik Roy, 3 years ago

Hi everyone, I would like to work on this ticket and already have started the process too. But I had one doubt while I was trying to solve the issue, URLreslove._is_callback() functionality also gets used in tests too. I am new to contributions and wanted to know should I also work on fixing tests too or just admindocs.py file and resolve.py file only.

Last edited 3 years ago by Alokik Roy (previous) (diff)

comment:5 by Alokik Roy, 3 years ago

Owner: changed from nobody to Alokik Roy
Status: new → assigned

comment:7 by Alokik Roy, 3 years ago

Has patch: set

comment:8 by Mariusz Felisiak, 3 years ago

Patch needs improvement: set

comment:9 by Alokik Roy, 3 years ago

Patch needs improvement: unset

Suggested Changes Implemented!

comment:10 by Carlton Gibson, 3 years ago

Needs documentation: set
Patch needs improvement: set

​Small comments on PR

comment:11 by Alokik Roy, 3 years ago

Needs documentation: unset
Patch needs improvement: unset

Updated the patch and documented the changes.

comment:12 by Carlton Gibson, 2 years ago

Triage Stage: Accepted → Ready for checkin

comment:13 by Carlton Gibson <carlton@…>, 2 years ago

Resolution: → fixed
Status: assigned → closed

In 7f3cfaa1:

Fixed #32565 -- Moved internal URLResolver view-strings mapping to admindocs.

Moved the functionality of URLResolver._is_callback(),
URLResolver._callback_strs, URLPattern.lookup_str() to
django.contrib.admindocs.

comment:14 by GitHub <noreply@…>, 2 years ago

In 974942a7:

Fixed #33955, Fixed #33971 -- Reverted "Fixed #32565 -- Moved internal URLResolver view-strings mapping to admindocs."

This reverts commit 7f3cfaa12b28d15c0ca78bb692bfd6e59d17bff1.

Thanks Tom Carrick and Greg Kaleka for reports.

comment:15 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

In a108380:

[4.1.x] Fixed #33955, Fixed #33971 -- Reverted "Fixed #32565 -- Moved internal URLResolver view-strings mapping to admindocs."

This reverts commit 7f3cfaa12b28d15c0ca78bb692bfd6e59d17bff1.

Thanks Tom Carrick and Greg Kaleka for reports.
Backport of 974942a75039ba43e618f6a5ff95e08b5d5176fd from main

comment:16 by Mariusz Felisiak, 2 years ago

Has patch: unset
Resolution: fixed
Status: closed → new
Triage Stage: Ready for checkin → Accepted

Patch was reverted.

Note: See TracTickets for help on using tickets.
Back to Top