Opened 5 years ago

Closed 5 years ago

#21330 closed New feature (wontfix)

More guidance on default settings in reusable applications is required

Reported by: Daniele Procida 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 Daniele Procida)

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 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, 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/
# myapp/
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.

Change History (7)

comment:1 Changed 5 years ago by Marc Tamlyn

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 5 years ago by Daniele Procida

Description: modified (diff)

comment:3 Changed 5 years ago by Daniele Procida

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 5 years 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 5 years ago by Daniele Procida

"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 5 years ago by Tim Graham

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 (not an endorsement, just the first result in Goggle) rather than the official docs.

comment:7 Changed 5 years ago by Tim Graham

Resolution: wontfix
Status: newclosed
Note: See TracTickets for help on using tickets.
Back to Top