Code

Opened 6 months ago

Closed 6 months ago

#21330 closed New feature (wontfix)

More guidance on default settings in reusable applications is required

Reported by: EvilDMP Owned by: nobody
Component: Documentation Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by EvilDMP)

There are numerous different ways of shipping reusable applications with sensible default settings so that the person reusing the application is not obliged to fill settings.py with numerous settings just to get started (if the application requires these settings).

Many of the easy-fix ways that this is done are poor, for example:

  • in settings.py, from app.default_settings import *
  • in app.default_settings, SETTING = getattr(settings, "SETTING", "value") (will cache settings values at module-level; breaks @override_settings)

and many are tremendously complex.

loic84's suggestion:

# myapp/settings.py
 
DEFAULT_MYAPP_WIDTH = 100
DEFAULT_MYAPP_HEIGHT = 100
 
# myapp/views.py
 
from django.conf import settings
from myapp.settings import DEFAULT_MYAPP_WIDTH
 
def view(request):
    print(getattr(settings, 'MYAPP_WIDTH', DEFAULT_MYAPP_WIDTH))

This is simple and robust enough to be recommended in the reusable apps tutorial, and should also be explained more fully along with the problem of module-level settings caching in the settings docs.

There should also be a note in the testing docs warning about how override_settings can easily be broken.

Attachments (0)

Change History (7)

comment:1 Changed 6 months ago by mjtamlyn

There are lots of possible ways to do this, and numerous community projects to provide tools to do it. Personally I don't think any of these are better or worse than each other, I don't think consistency is particularly important, and I'm not sure we should document a recommendation when a standard is not clear. The 'correct' solution to this likely depends on the pluggable app in question.

Take this as a -0 to discussing this in the docs, and likely a -1 to discussing it in a tutorial. How about you write your app so it doesn't need a ton of settings! Or better just has one - like DJ toolbar.

comment:2 Changed 6 months ago by EvilDMP

  • Description modified (diff)

comment:3 Changed 6 months ago by EvilDMP

That just seems like avoiding the problem though. Some applications do require project-wide settings, that aren't easily shoe-horned into database values or something else.

There are two parts to this:

  • warning about what not to do (many of the ways recommended to do it are simply wrong), so it's important to point out the pitfalls
  • providing at least one way to do it that may not be the best one for every single case, but works, is clear, and can be understood, so that the user can adopt or adapt it and at least get started

It's not enough for documentation to tell people what not to do, it also needs to give them a clue about what they should do instead - even if it's only a clue.

comment:4 Changed 6 months ago by loic84

I don't care all that much if we recommend a specific way to deal with settings in 3rd party apps, but I strongly believe we need to warn against the SETTING = getattr(settings, "SETTING", "value") at module level technique that I see way too often; it's plain bad.

comment:5 Changed 6 months ago by EvilDMP

"Recommendation" is a bit strong - we probably should not recommend a specific way to do it. But I do think that following the warnings we should show one good, safe way of doing it, if only as an example.

comment:6 Changed 6 months ago by timo

I guess I feel like Marc in that this seems a little out of place in terms of the docs we currently have for reusable apps, which is just the tutorial as far as I know. Did you have a place in mind for discussing this?

Happy to continue the discussion here, but I feel this would be more appropriate for something like http://django-reusable-app-docs.readthedocs.org/ (not an endorsement, just the first result in Goggle) rather than the official docs.

comment:7 Changed 6 months ago by timo

  • Resolution set to wontfix
  • Status changed from new to closed

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.