Code

Opened 6 years ago

Closed 14 months ago

#6412 closed New feature (fixed)

[patch] Check for file permissions for proper error messages

Reported by: mbeachy@… Owned by: MarkusH
Component: Core (Other) Version: master
Severity: Normal Keywords:
Cc: MarkusH 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@… 6 years ago.
simple debug.py patch to report non-readability of template files

Download all attachments as: .zip

Change History (22)

Changed 6 years ago by mbeachy@…

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

comment:1 Changed 6 years ago by SmileyChris

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Design decision needed

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

comment:2 Changed 6 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 3 years ago by julien

  • Type set to New feature

comment:4 Changed 3 years ago by julien

  • Severity set to Normal

comment:5 Changed 3 years ago by carljm

  • Easy pickings unset
  • Triage Stage changed from Design decision needed to Accepted
  • UI/UX unset

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

comment:6 Changed 21 months ago by lrekucki

  • Easy pickings set

comment:7 Changed 21 months ago by claudep

  • Needs tests set

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

comment:8 Changed 21 months ago by lrekucki

  • Owner changed from nobody to lrekucki

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 17 months ago by dokterbob

  • Owner changed from lrekucki to dokterbob
  • Status changed from new to assigned

Have merged with master and running tests right now.

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

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

Version 0, edited 17 months ago by dokterbob (next)

comment:10 Changed 17 months ago by dokterbob

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

comment:11 Changed 17 months ago by dokterbob

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

Todo: shorten commit line.

comment:12 Changed 17 months ago by dokterbob

  • Triage Stage changed from Accepted to Ready 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 17 months ago by dokterbob (previous) (diff)

comment:13 Changed 17 months ago by dokterbob

  • Triage Stage changed from Ready for checkin to Accepted

Better have a review first.

comment:14 Changed 16 months ago by MarkusH

  • 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 16 months ago by jezdez

  • Patch needs improvement set

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

comment:16 Changed 16 months ago by MarkusH

  • Owner changed from dokterbob to MarkusH

comment:17 Changed 16 months ago by MarkusH

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 16 months ago by MarkusH

  • Cc MarkusH added
  • Patch needs improvement unset

comment:19 Changed 14 months ago by MarkusH

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

comment:20 Changed 14 months ago by UloPe

  • Triage Stage changed from Accepted to Ready 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 14 months ago by Aymeric Augustin <aymeric.augustin@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 61a8de6f4f547518d217a5ff959cbd57bddf4bb0:

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

Report more details about template files in loader postmortem.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.