Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alternative approach to settings bootstrap #10

Open
markfinger opened this issue Feb 19, 2018 · 10 comments
Open

Alternative approach to settings bootstrap #10

markfinger opened this issue Feb 19, 2018 · 10 comments
Assignees

Comments

@markfinger
Copy link
Contributor

markfinger commented Feb 19, 2018

Rather than using split-settings to load a bunch of settings files, can we simply import the project_settings.py file which will explicitly name the settings modules imported?

# At the top of project_settings.py

from ixc_django_docker.settings.base.py import *
from ixc_django_docker.settings.compressor.py import *
from ixc_django_docker.settings.logentries.py import *
from ixc_django_docker.settings.storages.py import *
from ixc_django_docker.settings.whitenoise.py import *

The primary goal is reducing the magic so that we can get a more easily understandable ramp-up for new developers.

Also secondary benefits of making it easier for IDEs to jump to the settings file.

@markfinger
Copy link
Contributor Author

@jmurty: Tai indicated that a bunch of the changes to the settings system were a by-product of your input. Do you have thoughts here?

@markfinger
Copy link
Contributor Author

To clarify

a by-product of your input

I meant that they were made in response to issues that you had raised.

@mrmachine
Copy link
Collaborator

@markfinger @jmurty

I had a go at rewriting ixc_django_docker/settings/__init__.py as an actual settings module for a project... and I'm not convinced that where I ended up was any better...

  1. I ended up losing the useful (I think) verbose printing of what's actually being imported (in an attempt to keep the boilerplate to a minimum when it appears alongside actual project settings) and having to put most of the boilerplate up the top and some at the bottom (overrides); OR

  2. basically just reproducing the existing ixc_django_docker/settings/__init__.py as-is in project_settings/__init__.py and forcing a settings package (vs just a module) on the project, and then the only benefit is that you don't need to look into the venv to see which or how settings are loaded, but you do still need to look into the venv to see what the loaded settings do...

I think it's probably not worth it, although what I attempted is a little different than what @markfinger has suggested (vanilla imports)...

I do still think that using django-split-settings is the way to go, because using vanilla imports as suggested makes it impossible for the single purpose settings bundled with ixc-django-docker to alter settings that have come before it (e.g. base settings).

For example, you can't have a debug_toolbar.py that does INSTALLED_APPS += ('debug_toolbar', ) and then do from ixc_django_docker.settings.debug_toolbar import * or a develop.py that alters an existing setting dict. You end up with a really complex chain of imports to follow (what we had before), instead of an easy to digest list of files that are included in the order given, and each having access to the context of the ones that came before.

These are the reasons why we moved away from vanilla imports to django-split-settings in the first place, as ixc-django-docker's base settings were growing, becoming more tightly coupled, and making assumptions that aren't always true (e.g. that every project uses django-compressor).

I'd like ixc-django-docker to be able to offer solutions to the problems we encounter when running on ephemeral infrastructure and scaling horizontally, without forcing a project to use all of the packages we recommend to solve those problems, so a project can pick and choose or completely go it alone. I think composable single purpose settings modules help a lot with that.

And there's nothing stopping us from ignoring ixc-django-docker's default settings and doing anything you want in project_settings.py, if it really does get in the way. I've already done that in other projects, mostly just to keep the settings refactor to a minimum when dockerizing them (and pinched bits from ixc-django-docker where useful).

I'm currently more inclined to:

  • alter the default verbose output to be more concise or use logging instead to make it configurable (and make it as easy as uncommenting a line in .env.local.sample to disable or minimise) (to resolve Settings bootstrap process is noisy #8)

  • include a comment/link in the default/template project settings module pointing to a good diagram or docs about how settings are included, as @markfinger mentioned the existing comment was helpful and @jmurty has commented that he thinks a diagram would help

  • change the project template to use a package (vs just a module) -- I think for all but the most trivial of projects it is likely useful at least to be able to specify base/test/local settings as separate files, if not other envs (production, staging, develop), and doing this from the start solves should resolve Change local.py to project_settings_local.py #7 and remove the issue of starting simple with a single module then having to refactor into a package

@markfinger
Copy link
Contributor Author

How about we move the split-settings imports into the project's settings. Eg:

# At the top of project_settings.py
from split_settings.tools import include

include(
    'ixc_django_docker.settings.base',
    'ixc_django_docker.settings.compressor',
    'ixc_django_docker.settings.logentries',
    'ixc_django_docker.settings.storages',
    'ixc_django_docker.settings.whitenoise',
)

# project settings continue from here...

@mrmachine
Copy link
Collaborator

@markfinger that's what I tried, but you don't pass module named you pass file paths, and we need some extra logic to include dotenv or override settings (develop, test, staging, production), and we either lose the (I think) useful printout of what's actually being imported OR end up with a tonne of messy boilerplate in project settings...

import os

from split_settings.tools import include, optional
import ixc_django_docker.settings

# Directory containing ixc-django-docker settings modules.
IXC_DJANGO_DOCKER_SETTINGS_DIR = \
    os.path.dirname(ixc_django_docker.settings.__file__)

# Directory containing project settings modules.
PROJECT_SETTINGS_DIR = os.path.dirname(__file__)

# Settings file to include from both ixc-django-docker and project settings
# directories.
OVERRIDE_SETTINGS = os.environ.get('OVERRIDE_SETTINGS') or \
    '%s.py' % os.environ['DOTENV']

# Include `ixc-django-docker` settings.
include(os.path.join(IXC_DJANGO_DOCKER_SETTINGS_DIR, s) for s in (
    'base.py',
    # 'celery.py',
    # 'celery_email.py',
    'compressor.py',
    # 'extensions.py',
    # 'haystack.py',
    'logentries.py',
    # 'master_password.py',
    # 'nose.py',
    # 'post_office.py',
    # 'redis_cache.py',
    # 'sentry.py',
    'storages.py',
    'whitenoise.py',
))

# Include `ixc-django-docker` override settings.
include(optional(
    os.path.join(IXC_DJANGO_DOCKER_SETTINGS_DIR, OVERRIDE_SETTINGS)))

# Include project settings OR replace with your actual project settings.
include(os.path.join(PROJECT_DIR, s) for s in (
    'base.py',
    'foo.py',
))

# Include project override settings.
include(optional(os.path.join(PROJECT_SETTINGS_DIR, s)) for s in (
    OVERRIDE_SETTINGS,
    'local.py'
))

Is this really any better than just having a comment that points you to docs or a diagram or ixc_django_docker/settings/__init__.py to discover how it works? You still have to jump into the venv to see what the base and optional ixc-django-docker settings modules do.

Like I said, there's absolutely nothing stopping you from implementing exactly this (or something else) in your project without making any change to ixc-django-docker. You might need to set DJANGO_SETTINGS_MODULE=project_settings in .env.base to completely disable ixc_django_docker/settings/__init__.py.

But I think that it's important for ixc-django-docker to provide a usable settings module out of the box that enables/includes settings that solve the ephemeral/scaling issues that come with running Django on Docker, even if only to be used as a reference example.

But, I do think I can make it do a better job in looking for local overrides when given a Python module instead of a Python package as DJANGO_SETTINGS_MODULE, to fix #7

@markfinger
Copy link
Contributor Author

Nah, that's fair enough. We've attempted the easy workarounds, and it seems like none are viable.

I'm happy to roll with things

@mrmachine
Copy link
Collaborator

@markfinger Here's where I've tried to explain simply how to configure the settings that get included: https://github.com/ixc/ixc-django-docker/blob/6d7191679fe26a42871050d84d6f790087015a4a/README.rst#about-settings-modules

Would linking to this have been better than linking to ixc_django_docker/settings/__init__.py in a code comment?

@jmurty
Copy link
Contributor

jmurty commented Feb 20, 2018

I don't have much to add here. My only other thought about how to make the settings less opaque from within a project would be to have a more explicit project settings file template in ixc-jango-docker that could be imported into a project and optionally re-imported if/when there are updates to ixc-jango-docker that you want to use in the project.

But I am tending towards trying to solve the inherent complexity of the settings with clear and easily-found documentation, rather than shifting it around. I don't think there is a way to eliminate it, just hide it more or less well.

@jmurty
Copy link
Contributor

jmurty commented Feb 20, 2018

A good starting point for documenting the settings might be a relatively simple table describing each setting module/file, what it does, and the settings you might most often need to tweak with clear guidance on where to apply the tweak (before or after the default settings are loaded)

@markfinger
Copy link
Contributor Author

@mrmachine

Would linking to this have been better than linking to ixc_django_docker/settings/init.py in a code comment?

Yeah, sounds good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants