dotkom / onlineweb4

Web pages for Online, Linjeforeningen for Informatikk at NTNU
https://online.ntnu.no
MIT License
48 stars 23 forks source link

Separate settings files #976

Closed melwil closed 6 years ago

melwil commented 10 years ago

We have a few apps that are in separate repositories from ow4, and might require more than a few settings. Would it be cleaner to have these as separate settings files?

My motivation for suggesting this is that base.py gets quite cluttered after a while, and should perhaps include most settings that are connected to django directly. 3rd party apps that I consider taking out settings for in the first place are the ones that are their own systems, like feedme, wiki and redwine.

There are two ways I think this could be done:

base.py is on 333 lines right now, and we could easily take out around 50 of these into its own file, perhaps making this worth it in the long run. Any thoughts?

myth commented 10 years ago
if 'blabla' in INSTALLED_APPS:
    from app.settings import *
sklirg commented 10 years ago

Taken from feedme repo:

Add the following to settings: FEEDME_GROUP = 'feedme_users_group' FEEDME_ADMIN_GROUP = 'feedme_admins_group'

Would one rather create feedme/settings.py and add those lines to that file?

What if the system already has these settings set? Will they be overwritten?

hernil commented 9 years ago

I think a well structured addons-py would be a good idea.

christiansyoung commented 9 years ago

I have seen some convention on this in other projects that has overriding settings in their own files. Such as settings/filebrowser.py

myth commented 9 years ago

Keeping a settings file in the app is totally fine. They will not override settings from django unless specifically imported in the settings file set to be the default config for the django instance.

If external settings is required, a good convention is to provide defaults, and if collisions are detected, settings from the central settings.py are used, otherwise, app-local settings are used. But there are many ways of accomplishing this functionality that are equally good.

sklirg commented 9 years ago

I'm pretty sure we had a chat about this recently, and while base.py gets bigger and bigger, adding new empty lines of code to example-local.py is a bad way for developers to receive updates. (Who ever overwrites their local.py?

I suggest splitting settings files into apps, or at least separate KEYS out of local.py, into something like keys.py. This file can contain all keys required, e.g. we recently added RECAPTCHA keys, but no one probably noticed (I didn't..) because they are in a file you copy once when you set up the project and then customize to your needs (for logging etc.), and should never have to update again.

melwil commented 9 years ago

How about defaults.py for all the settings you would need in dev? And then:

if debug: 
    from defaults import *

The reason we have local.py is not for development, but rather for the production environment, where we want hidden keys and such that are outside the repo, but still in the structure of the code. If you copy example-local.py when you set up the project, that creates this dummy file, but any settings the project needs would actually be somewhere else.

local.py could still contain stuff like dev emails, dev db, logger settings and so on, but defaults.py would contain any value the project needs to run smoothly beyond the obvious.

sklirg commented 9 years ago

Yup! Sounds good to me.

:+1:

This means we can stop requiring local.py as well, I guess! (some whining happens if you haven't copied example-local to local.py)

sklirg commented 8 years ago

I'm working on a project where we tried to separate settings files. A problem emerged and there's no nice solution. How should you proceed if you want to use a setting in a settings file, if it's overridden in local.py?

Case: The site URL, which by default probably will be set to https://online.ntnu.no, and will be overridden in local.py to http://127.0.0.1:8000. If there's a setting in e.g. staticfiles.py which depends on this URL to properly serve static files for the given domain, you will use the setting set in base.py, since you've not yet loaded local.py which is supposed to override this setting.

In the aforementioned project, we had this case:

# base.py
BASE_URL = 'example.org'  # our domain

# media.py
from project.settings import BASE_URL
MEDIA_URL = urljoin(BASE_URL, 'media/')

# local.py
BASE_URL = '127.0.0.1:8000'

The expected output for MEDIA_URL is localhost:8000/media/, but since BASE_URL was example.org when media.py-settings were loaded, it was set to example.org/media/, which does not resolve to the current environment.

One "nice hack" is to manually re-load the affected settings in local.py after we're done loading settings. Or we can load local.py before the affected settings (but then we'd loose the overriding potential for later files - unless we make multiple local.py's).


To suggest a solution to the issue at hand;

Format the file using markdown. Maybe we could have Terms of Content at the top. Would be nice if it was alphabetical as well, and/or follow the INSTALLED_APPS list.

# Onlineweb 4 project settings
## Terms of Content

- Django settings
 - Databases
 - Staticfiles
 - Templates
- 3rd party app settings
 - Django REST framework
 - Stripe

## Django settings
### Database settings
DATABASES = {
    'default': {
        'name': 'django',
        'user': 'django',
        'password': 'django',
}

## 3rd party app settings
### Django REST framework
REST_FRAMEWORK = {
    'DEFAULT_PAGINATION_CLASS': 'rest_framework.pagination.PaginationClass',
}

### Stripe
STRIPE_KEYS = {
    'prokom': '1337haxx0r',
}

## OW4 apps settings
### Payment
WHATEVER_SETTINGS_WE_HAVE = True

Whenever I look for something in base.py, I search for it. Ctrl + f. It doesn't matter how "big" the file is (This would become harder if the settings files are split, especially when not in an IDE). Where should we split the settings files, how many files do we want, one for each app that has settings? It can very quickly become messy with splitting the up as well.

EDIT: Here is a nice wiki article regarding "SplitSettings". I'd like to quote most of the configurations;

Keep application wide, unsensitive settings and sane defaults in your normal settings.py file.

myth commented 8 years ago

I have come to love the use of just a single settings file for the project as a whole. Apps that are so large and/or complex that they need their own settings file usually keep it in the app-directory. The nifty thing with that is that if conflicts are detected, django prefers settings from the main settings.py over the app specific.

And to resolve the issues of sensitive values and values needing to be replaced, set environment variables in the virtualenv that the application is being run in. By doing so, each setting in the one and only project settings file is fetched via a get-or-default scheme from the environment. This also makes it quite easy to write a simple sanity-check function that iterates over all attributes in the settings module, looking for missing flags. It could then halt execution, giving a warning to console that a critical setting has not been set. I've played around a bit with this approach and have grown to love it.

I'm not saying it is the one and only go-to solution, but it certainly works. Thoughts?

sklirg commented 8 years ago

What do you mean by this?

This also makes it quite easy to write simple sanity-check function that iterates over all attributes in the settings module, looking for missing flags. It could then halt execution, giving a warning to console that a critical setting has not been set.

In production, in development? What characterises as a "critical setting"? Why not just have some "sane defaults" (which makes the app run), and make sure to override then once-and-for-all (why would you ever need to change them?) in production/staging?

myth commented 8 years ago

All things that we currently have to override in local.py is a setting that would fall under that category. Using development as a default environment for operation, a sane default that lets the application run can of course just be SOME_KEY=os.getenv('SOME_KEY', 'development'). If however a production flag is set in the environment, all settings module attributes containing placeholder values will trigger a halt in execution, with a friendly reminder to update SOME_KEY in the bootstrap script, virtualenv file or other appropriate place.

This keeps all settings in one file, no need for copy and paste. You will be reminded if you are forgetful when pushing stuff to production, and you will hopefully never have to tinker with settings to get development env up and running.

sklirg commented 8 years ago

So for any setting in local.py (or example-local.py), we'll use this syntax; MY_NEW_KEY = os.getenv('MY_NEW_KEY', 'sane default setting') ? Sounds simple, I like it.

myth commented 8 years ago

Yes, except that there only needs to be settings.py.

sklirg commented 8 years ago

I love that.

myth commented 8 years ago

This also allows you to lets say, put some env flag checks for stuff that needs to be overridden in build/test environments.

if os.getenv('TEAMCITY', False):
    SOME_SETTING = 'build_dependant_value'
myth commented 8 years ago

I made a write-up when setting up my project and envs from scratch last night, which also includes what i mean by using environment variables to handle sensitive settings and other stuff, for those interested (you can skip to the Django settings chapter).

duvholt commented 6 years ago

This should be done now that we have settings based on environment variables and separate settings files for some packages.