Amending initial settings

[daaray]

• Add django-dotenv support to move more toward common django/wagtail conventions WRT settings file
• add support for local.py settings file
• fix vagrant db issues by building abs path to sqlite db file
• adjust readme accordingly

[daaray]

@heymonkeyriot @shacker @ArnarTumi

Guys, this PR changes setup a bit, but is more inline with current standards. I will refrain from merging until I can get one or more sets of your eyes on it.

[hminnovation]

I've personally never used dotenv before, but this certainly looks like a more elegant way to handle those variables. Playing devil's advocate the only question I have is whether that elegance justifies introducing a new piece of tooling? I think it probably does, just want to double check.

It's great to have 'NAME': os.path.join(BASE_DIR, 'bakerydemodb'), that's definitely going to make the db bit of the settings more robust.

Would be interested to hear what @shacker and @ArnarTumi think too.

[daaray]

A few additional comments:

• Importing local settings and dev settings is an antipattern
• dotenv allows the app to abide more closely to 12 Factor app development

Both of the above lead me to believe that the extra dependency (and additional docs) warrant the change, as it will encourage better development practices.

[shacker]

I'm going to disagree with the idea of using a dotenv, for these reasons:

• This demo site doesn't even need any localized settings. It runs fine with a single global settings file. For that reason, I'd actually suggest we switch from base.py to standard settings.py
• The only change a person might make would be if they wanted to use postgres. But even then, they can change settings.py just as easily
• If a person wants to use this as a starter site rather than as an educational tool (which I don't think is a good idea - they'd end up ripping out almost everything provided here and replacing it with their own models, templates, etc.), they'll want to implement local settings of some sort, but that's up to them to solve via the Django docs; not our problem to solve
• People question whether the dotenv suggestion in 12-factor should even be on that list. How are environment vars "better" than a local config file? Now you've got some settings in python and others in bash. To what end? That's just complication you don't need. local.py does exactly the same job. Perhaps we should add local.py and local_settings.py to .gitignore just in case they do?
• We want to avoid every additional piece of tooling possible. Our job is to teach about wagtail, not to become a model for every best practice out there, especially when some of those (like this one) are controversial (read the discussion in this gist).

I think dotenv adds to the noise and doesn't really advance the ball for the user.

[shacker]

The important thing is to keep local settings out of the VCS, and that's trivially easy to do with local.py via .gitignore. But again, how to do that is up to each site (and has nothing to do with wagtail).

[daaray]

@shacker This approach might make more sense now that the Heroku PR was merged into master and subsequently into this branch.

Heroku provides a push button deploy. For those individuals that want/need to try the demo locally and have a further need to make their demo web accessible, this path has the least resistance (and was supported in the previous demo).

Unfortunately, Heroku:

• Does not support the notion of local.py at all
• Does not support sqlite as a db backend

The previous demo worked around this issue by forcing postgres on the user, utilized a non-DRY pattern for settings, and commingled All The Requirements in one file for both local and Heroku support.

I completely agree that the primary goal of this repo is to provide an educational tool for Wagtail. But, I also believe that we should abide by conventions and best practices du jour as a secondary goal. To be fair, I may be biased by the patterns we utilize at work.

If there is an alternative suggestion to allow for support of local and Heroku deploys, I am all ears. A compromise might be to move the docs regarding local env var setting to a separate section and adjust the manage.py file to look for settings.base be default. Thoughts?

[shacker]

Oh, interesting - I didn't realize Heroku didn't support having a local.py. Although this article implies that it is supported (but also says that's not the most up to date approach).

Well, now I'm torn. I personally don't see any value to having this demo site run on Heroku, or in us trying to solve deployment challenges for the user - it all feels like a big distraction from our purpose. But am pretty sure I'm in the minority with this opinion. So, IF we're going to maintain Heroku support, AND Heroku doesn't like local.py AND Heroku doesn't support sqlite, then yes, we need to change up the way we're doing things. I don't want to have two approaches - one for people who want to deploy and another for those who don't - so we need to consolidate.

It sounds to me like Heroku support will force our hand into moving to postgres and using dotenv.

Choices:

• Keep Heroku support and switch to postgres + dotenv
• Ditch Heroku support and keep it simple / focused on Wagtail itself, not deployment

Show of hands?

[gasman]

+1 for Heroku. People do use the deploy-to-Heroku button on the current wagtaildemo (and complain when it breaks :-) ), and it's somewhat likely that we'll choose Heroku for hosting a public demo site (a long time wishlist item that's been put off because we didn't have a demo app that was up to scratch...)

[hminnovation]

I think it's important that we support users who want to be able to deploy this. I think we're assuming a lot of knowledge if we think someone coming fresh to Wagtail will be able to get it deployed without some direction. Speaking for myself coming from working on PHP, with standard LAMP stack thinking, everything about deploying Wagtail initially confounded me, and the old demo site didn't help me much because of the co-mingling of the settings as @daaray mentioned.

Locally though we can presumably maintain sqlite rather than Postgres? It certainly speeds up the initial local build time.

Whilst I think it may be out side the scope of this initial build working out how we can deploy in a pain-free way for those non-technical users to see the admin is something we should have in the back of our heads too. This feels like it would facilitate us in doing that down the line.

I think then my vote goes on the keeping of dotenv side of things.

[daaray]

We can address any remaining issues in followup tickets. Thanks for the discussion!