Opened 9 years ago

Closed 3 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: master
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@… 9 years ago.
simple debug.py patch to report non-readability of template files

Download all attachments as: .zip

Change History (22)

Changed 9 years ago by mbeachy@…

Attachment: tmp.patch added

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

comment:1 Changed 9 years ago by Chris Beaven

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedDesign decision needed

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

comment:2 Changed 9 years ago by Simon Willison

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

comment:3 Changed 5 years ago by Julien Phalip

Type: New feature

comment:4 Changed 5 years ago by Julien Phalip

Severity: Normal

comment:5 Changed 5 years ago by Carl Meyer

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 Changed 4 years ago by Łukasz Rekucki

Easy pickings: set

comment:7 Changed 4 years ago by Claude Paroz

Needs tests: set

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

comment:8 Changed 4 years ago by Łukasz Rekucki

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 Changed 4 years ago by Mathijs de Bruin

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 4 years ago by Mathijs de Bruin (previous) (diff)

comment:10 Changed 4 years ago by Mathijs de Bruin

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

comment:11 Changed 4 years ago by Mathijs de Bruin

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

Todo: shorten commit line.

comment:12 Changed 4 years ago by Mathijs de Bruin

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/837

Version 0, edited 4 years ago by Mathijs de Bruin (next)

comment:13 Changed 4 years ago by Mathijs de Bruin

Triage Stage: Ready for checkinAccepted

Better have a review first.

comment:14 Changed 4 years ago by Markus Holtermann

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 Changed 4 years ago by Jannis Leidel

Patch needs improvement: set

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

comment:16 Changed 4 years ago by Markus Holtermann

Owner: changed from Mathijs de Bruin to Markus Holtermann

comment:17 Changed 4 years ago by Markus Holtermann

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 Changed 4 years ago by Markus Holtermann

Cc: Markus Holtermann added
Patch needs improvement: unset

comment:19 Changed 3 years ago by Markus Holtermann

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

comment:20 Changed 3 years ago by Ulrich Petri

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 Changed 3 years ago by Aymeric Augustin <aymeric.augustin@…>

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