Opened 16 years ago

Closed 11 years ago

#6412 closed New feature (fixed)

[patch] Check for file permissions for proper error messages

Reported by: mbeachy@… Owned by: Markus Holtermann
Component: Core (Other) Version: dev
Severity: Normal Keywords:
Cc: Markus Holtermann 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

This is a duplicate to #1194 with a simpler fix that only affects debug.py.

This problem bit me earlier today (in my first django installation) and while it wasn't *that* hard to figure out it would've saved me a good 20 minutes of poking around. Think of the newbies!

Also, FWIW the non-readability of templates was caused by a too-restrictive umask (0027) on my part; apparently data file installation from setup.py copies without modifying permissions, so I was stuck.

Attachments (1)

tmp.patch (1.4 KB ) - added by mbeachy@… 16 years ago.
simple debug.py patch to report non-readability of template files

Download all attachments as: .zip

Change History (22)

by mbeachy@…, 16 years ago

Attachment: tmp.patch added

simple debug.py patch to report non-readability of template files

comment:1 by Chris Beaven, 16 years ago

Triage Stage: UnreviewedDesign decision needed

Interesting idea. Seems like a good one, but I'll pass via design decision.

comment:2 by Simon Willison, 16 years ago

I spent half an hour debugging this exact problem a while ago - having the debug view highlight it is an excellent idea.

comment:3 by Julien Phalip, 13 years ago

Type: New feature

comment:4 by Julien Phalip, 13 years ago

Severity: Normal

comment:5 by Carl Meyer, 13 years ago

Easy pickings: unset
Triage Stage: Design decision neededAccepted
UI/UX: unset

This seems like a useful feature that can be added in just a few lines of code.

comment:6 by Łukasz Rekucki, 11 years ago

Easy pickings: set

comment:7 by Claude Paroz, 11 years ago

Needs tests: set

Looks good, but I still think that this should be tested.

comment:8 by Łukasz Rekucki, 11 years ago

Owner: changed from nobody to Łukasz Rekucki

Eh, I have a hard time testing this on Windows (can't set a file unreadable). Will update the patch next week.

comment:9 by Mathijs de Bruin, 11 years ago

Owner: changed from Łukasz Rekucki to Mathijs de Bruin
Status: newassigned

Have merged with master and running tests right now: https://travis-ci.org/dokterbob/django/builds/5020749

https://github.com/dokterbob/django/tree/ticket6412

Note: Yes, Travis support will be rebased out in the definitive patch.

Last edited 11 years ago by Mathijs de Bruin (previous) (diff)

comment:10 by Mathijs de Bruin, 11 years ago

Tests are failing on Python 3.x due to errors with StringIO. Attempting to fix now.

comment:11 by Mathijs de Bruin, 11 years ago

Waiting for https://travis-ci.org/dokterbob/django/builds/5021135

Todo: shorten commit line.

comment:12 by Mathijs de Bruin, 11 years ago

Triage Stage: AcceptedReady for checkin

Build passed: https://travis-ci.org/dokterbob/django/builds/5021135

Fixed import issue in Python 3.x related to StringIO import in the process.

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

Last edited 11 years ago by Mathijs de Bruin (previous) (diff)

comment:13 by Mathijs de Bruin, 11 years ago

Triage Stage: Ready for checkinAccepted

Better have a review first.

comment:14 by Markus Holtermann, 11 years ago

Needs tests: unset

I tested the linked pull request (https://github.com/django/django/pull/838) on Python 2.7.3 and Python 3.3.0 with Linux. The tests pass for both versions.

comment:15 by Jannis Leidel, 11 years ago

Patch needs improvement: set

The patch looks almost good, although using a NamedTemporaryFile would be better.

comment:16 by Markus Holtermann, 11 years ago

Owner: changed from Mathijs de Bruin to Markus Holtermann

comment:17 by Markus Holtermann, 11 years ago

I updated the patch to use the NamedTemporaryFile for the former two tests. The latter works on a directory, hence there is no way to automatically remove that directory.

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

comment:18 by Markus Holtermann, 11 years ago

Cc: Markus Holtermann added
Patch needs improvement: unset

comment:19 by Markus Holtermann, 11 years ago

I updated the pull request to fix some PEP-8 issues. https://github.com/django/django/pull/947

comment:20 by Ulrich Petri, 11 years ago

Triage Stage: AcceptedReady for checkin

Patch looks good. I would prefer it if the messages were not defined in the view. But since this is the debug view the template is included in the same file anyway so this is no big deal.

comment:21 by Aymeric Augustin <aymeric.augustin@…>, 11 years ago

Resolution: fixed
Status: assignedclosed

In 61a8de6f4f547518d217a5ff959cbd57bddf4bb0:

Fixed #6412 -- More details if a template file cannot be loaded

Report more details about template files in loader postmortem.

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