Skip to content
This repository has been archived by the owner on Jan 13, 2022. It is now read-only.

12-factor: Get config variables from the environment. #145

Merged
merged 3 commits into from Aug 14, 2015

Conversation

pmclanahan
Copy link

Add dj-database-url, django-cache-url, and python-decouple deps.

Add dj-database-url, django-cache-url, and python-decouple deps.
@pmclanahan
Copy link
Author

@jgmize more 12factorization when you get time. I feel like maybe you've looked at some of this already, but I could be wrong.

default='.allizom.org, basket.mozilla.com, basket.mozilla.org',
cast=Csv())
SESSION_COOKIE_SECURE = config('SESSION_COOKIE_SECURE', True, cast=bool)
CSRF_COOKIE_SECURE = config('CSRF_COOKIE_SECURE', True, cast=bool)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally prefer the explicit kwarg default=True to the positional True for readability, but either way works.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After looking at a big block of defaults lower down, I changed my mind. I prefer your way now :)

@jgmize
Copy link

jgmize commented Aug 14, 2015

Yes, I had looked at most of this before. I didn't see any blockers to merge into the 12factor branch; your call on whether it would be better to provide defaults for the settings I mentioned.

r+, nice work :)

CACHES = {
'default': config('CACHE_URL',
default='locmem://',
cast=django_cache_url.parse)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you checked out django-cache-url? It seems quite good, and supports django-redis which I plan on using in the deis deploy.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And for multiple servers you just have to comma separate them. e.g. memcached://server1:11211,server2:11211

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you checked out django-cache-url? It seems quite good, and supports django-redis which I plan on using in the deis deploy.
And for multiple servers you just have to comma separate them. e.g. memcached://server1:11211,server2:11211

I looked at it before and liked it, but somehow I missed the option to specify multiple servers with commas. Nice.

@pmclanahan pmclanahan force-pushed the 12factor-settings branch 2 times, most recently from a17239c to e606008 Compare August 14, 2015 16:23
@pmclanahan
Copy link
Author

Settings moved. Feels good. If you agree I'll merge this (or just FF merge) into 12factor and start I think on removing vendor and using peep. That will feel amazing :)

@pmclanahan pmclanahan merged commit 9338a37 into mozilla:12factor Aug 14, 2015
@pmclanahan pmclanahan deleted the 12factor-settings branch August 14, 2015 17:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants