Skip to content

allow for CONTENT_ROOT to be set with an environment variable,#5189

Merged
benjaoming merged 1 commit intolearningequality:masterfrom
xaiki:master
Jun 28, 2016
Merged

allow for CONTENT_ROOT to be set with an environment variable,#5189
benjaoming merged 1 commit intolearningequality:masterfrom
xaiki:master

Conversation

@xaiki
Copy link
Contributor

@xaiki xaiki commented Jun 28, 2016

Summary

allow for CONTENT_ROOT to be set with an environment variable

TODO

If not all TODOs are marked, this PR is considered WIP (work in progress)

  • Have tests been written for the new code? If you're fixing a bug, write a regression test (or have a really good reason for not writing one... and I mean really good!)
  • Has documentation been written/updated?
  • New dependencies (if any) added to requirements file

Reviewer guidance

If you PR has a significant size, give the reviewer some helpful remarks

Issues addressed

List the issues solved or partly solved by the PR

Documentation

If the PR has documentation, link the file here (either .rst in your repo or if built on Read The Docs)

Screenshots (if appropriate)

They're nice. :)

We use this at endlessm.com to allow sharing content whilst keeping the databases separate (we merge them on launch).

Signed-off-by: Niv Sardi xaiki@evilgiggle.com

We use this at endlessm.com to allow sharing content whilst keeping the databases separate (we merge them on launch).

Signed-off-by: Niv Sardi <xaiki@evilgiggle.com>
@benjaoming
Copy link
Contributor

Related to #5144 I guess -- and thanks for another contribution.

It seems reasonable, but if you want more, I would suggest a custom settings module, that scales much better than having to do a PR everytime ;)

Since KA Lite's settings are generally unmanagable, hardly documented and scattered, it seems like adding to worse by wrapping oddly chosen stuff in os.getenv :)

@benjaoming benjaoming merged commit 0cab5a8 into learningequality:master Jun 28, 2016
@benjaoming
Copy link
Contributor

Btw. because of the way settings are derived from CONTENT_ROOT, then there is some idea to setting it in an env var.

The method using a settings module is somewhat described in the docs, namely by explicitly setting all the derived settings as well! :)

@benjaoming
Copy link
Contributor

Oh sorry, didn't see this was going into master

@benjaoming
Copy link
Contributor

@xaiki - you have to make the PR for the branch named 0.16.x, then we can have it released for 0.16.7.

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

Successfully merging this pull request may close these issues.

2 participants