Code

Opened 4 years ago

Closed 22 months ago

Last modified 10 months ago

#12493 closed Cleanup/optimization (fixed)

settings.configure silently allows TEMPLATE_DIRS configuration error

Reported by: mckoss Owned by: gwilson
Component: Core (Other) Version: 1.1
Severity: Normal Keywords: settings, templates
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

settings.configure(TEMPLATE_DIRS="/path/to/a/single/dir")

Result:

Silently accepted, even though the code is expecting an array of path strings.

Expect:

Either a) generate an error message when invalid settings are set, or b) allow this setting to be either a single directory OR a sequence
of directories.

Note that configuration errors like this can be very painful to track down - as the site of the error is quite far from the results (in this case, the template system is unable to find the file ... but there is no indication as to why that is the case w/o debugging the Django sources - which is what I did finally to figure out my mistake).

Attachments (0)

Change History (8)

comment:1 Changed 4 years ago by ramiro

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

A workaround for this kind of user errors is already present since r213 for the case of settings read from a settings module. Should something similar also be done for the settings.configure() caseor can we close this assuming the user using such advanced scheme should make sure to read the relevant setting documentation?. Setting triage status to 'Design decision needed'.

comment:2 Changed 4 years ago by Alex

Ugh, I'm opposed to magically fixing those settings as it is now, obviously we can't undo those (yay backwards compatibility), but I don't like extending them further.

comment:3 Changed 4 years ago by lukeplant

I agree with Alex - fixing them automatically is pretty nasty, because it means that developer is not alerted about their error. If they set TEMPLATE_DIRS incorrectly in their settings.py, and use the value somewhere else (e.g. as part of another setting, or in some other code), the possible consequences violate the principle of least astonishment. Throwing a validation error would have been much better IMO.

I'm also not convinced the error is that inscrutable. In the normal case, you get a debug page with the error TemplateDoesNotExist. The TEMPLATE_DIRS setting, which is the obvious thing to check, is displayed at the bottom of the page, and will show a single string — and I think it ought to be fairly obvious from the plural name TEMPLATE_DIRS that the setting should be an iterable of some kind.

In the context of a settings file, you are dealing with people who might not know Python, and the need for the trailing comma in a one-tuple is quite an obscure syntax point to expect non-Python developers to know. That argument isn't so strong for normal Python code.

comment:4 Changed 3 years ago by julien

  • Severity set to Normal
  • Type set to Cleanup/optimization

comment:5 Changed 3 years ago by gwilson

  • Easy pickings unset
  • Owner changed from nobody to gwilson
  • Status changed from new to assigned
  • UI/UX unset

Also agree that this should be raising an error instead of auto-correcting, and should perhaps be considered for removal. However, the fact remains that this is a bug since the behavior using settings.configure() is not the same as the standard behavior.

comment:6 Changed 2 years ago by aaugustin

  • Triage Stage changed from Design decision needed to Accepted

Accepting as explained by gwilson. We'll need a deprecation path for the switch from auto-correcting to raising an exception.

comment:7 Changed 22 months ago by Claude Paroz <claude@…>

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

In [db87016b1a92b790c4250a2b5c16ceadd737a00e]:

Fixed #12493 -- Deprecated auto-correction of TEMPLATE_DIRS

comment:8 Changed 10 months ago by Ramiro Morales <cramm0@…>

In 64cdea68e71829905da6374a066d1700375255ec:

Report wrongly-typed settings and abort, as originally planned.

Thanks Claude for the heads up. Refs #12493 and commit 5e08b792.

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.