#5904 closed Cleanup/optimization (wontfix)
reverse should be more lenient on non-existent views in the urlconf
| Reported by: | Chris Beaven | Owned by: | nobody |
|---|---|---|---|
| Component: | Core (URLs) | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Accepted | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | yes | Patch needs improvement: | yes |
| Easy pickings: | no | UI/UX: | no |
Description
If you try to use the reverse (even for an existing view) while you have a non-existent view referenced in your urlconf, a ViewDoesNotExist error will be raised.
This happens because a reverse_dict cache is built for the RegexURLResolver containing a list to all pattern callbacks and names.
Rather than the whole thing falling over, all that should really happen in this case is that the callback is left out of reverse_dict.
Attachments (1)
Change History (11)
by , 18 years ago
| Attachment: | reverse_bug.diff added |
|---|
comment:1 by , 18 years ago
| Triage Stage: | Unreviewed → Ready for checkin |
|---|
comment:2 by , 18 years ago
| Resolution: | → wontfix |
|---|---|
| Status: | new → closed |
Firstly, not ready for checkin, since it doesn't include a test (which would have been one line long).
More importantly, this is covering up bugs in peoples' code. If you are referencing a view that doesn't exist, it's equivalent to having invalid syntax in your Python source. We shouldn't be continuing on in that case, regardless of whether the bug is nearby or just in a referenced file. Raising an exception is the right thing to do.
If somebody wants to make improvements in this area, come up with a way to report such errors back to the developer when the {% url %} tag is used (not via the template). Don't pretend the bug in their code doesn't exist.
comment:3 by , 17 years ago
| Resolution: | wontfix |
|---|---|
| Status: | closed → reopened |
In the {% url %} tag, which also raises this error it is very annoying. The default for non existant variables in template syntax is to silently ignore the problem, this should also occur for views that don't exist yet (whether relevant or irrelevant).
Secondly it is bad that an error in a different application can cause an error in the current application, as they are supposed to be independent. There is no reason to raise this error ever, and particularly not for parts of the code that don't affect the result of the call.
comment:4 by , 17 years ago
| Patch needs improvement: | set |
|---|---|
| Triage Stage: | Ready for checkin → Accepted |
It's reasonable that the url tag should catch the error and do nothing -- that's normal template behaviour and not doing so is a bug. A patch to fix that would be good.
However, the behaviour of reverse() with respect to the error reporting it does now should be unchanged. It's an error to refer to invalid view files. Equivalent to having syntax errors in your code or randomly deleting files. There's simply no way to argue otherwise. You might hope for less draconian error handling, but that would be disguising symptoms. The correct fix is to fix the bugs in the code that causes the problem (it takes two seconds to comment out a line that isn't implemented yet, for example). So no change to reverse() behaviour is required.
comment:5 by , 15 years ago
| Needs tests: | set |
|---|---|
| Severity: | → Normal |
| Type: | → Cleanup/optimization |
#6379 was closed as dupe and has a patch. While I'm here: nothing is actually broken so I'll call this an optimization. Things have changed in the last 3 years though, so I'm not sure if the issue still exist.
comment:8 by , 13 years ago
| Status: | reopened → new |
|---|
comment:9 by , 10 years ago
| Resolution: | → wontfix |
|---|---|
| Status: | new → closed |
By deprecating string views in urlpatterns, I don't think we'll have the problem of referencing non-existent views.
comment:10 by , 10 years ago
| Component: | Core (Other) → Core (URLs) |
|---|
I'll be brash and promote straight to checkin