moodle-docker
moodle-docker copied to clipboard
Add support for reading a .env file
This commit adds support for a .env file to provide default environment configuration using the shdotenv project with an environment file in the POSIX format.
I have not (yet) been able to find a Window variant of shdotenv, and I'm not sure if there's already something built-in to Powershell or similar so this remains a Linux/MacOS feature for now.
By any chance did you see https://github.com/moodlehq/moodle-docker/pull/232 ?
So... the order of precedence (fallback if not defined) for env variables is:
- Already defined (via export).
- dotenv file.
- moodle-docker default.
Correct? And the fallback won't ever "overwrite" already existing ones.
Pity we cannot do this under Windows, we also try hard to keep feature-parity.
Note I'm also looking to #232, that doesn't require any external stuff (surely it's a little more "fragile" about what can process, but also, surely, what it supports is enough for moodle-docker) and comes with Windows support.
Ciao :-)
Hi @andrewnicols , Thanks for working on this. I’ve given it a review. I agree with @stronk7 points.
Pros: simple change Cons: doesn’t work on windows, uses a third party app (that isn’t very active)
I’ve read the shdotenv readme and understand their rationale. I’m not sure it applies so much in this case, as this is a dev tool. I’d probably prefer a solution that works in windows (as well as mac and linux), and that doesn’t require extra dependencies, over one that’s a bit more secure (again, only because this is a dev tool).
In this regard the proposed solution in #232 is a bit better. However, I don’t think it’s quite ready either. I’ll update #232 with similar feedback to here, but I think the use of source
to include another bash file is an abstraction that doesn’t add a huge amount of value. I’d be happy for the logic to be all in the moodle-docker-compose
wrapper. There are a couple of other small things that I’ll add to my review of that issue.
So I think the way forward here is go with #232 plus the suggested changes. Or update this issue to implement it in a similar way. I’m keen to see this functionality merged in.
Cheers, Matt P
Just for the record, I don't see much value in either solutions TBH, they seem like an overkill for the simple env vars loading from the file.
I am using very simple approach for this. I have a folder with collections of env files and I load the one I currently need for tests. For example, the content of /home/user/docker/moodle-docker.env
may look like this:
# Set up path to Moodle code
export MOODLE_DOCKER_WWWROOT=/home/user/git/moodledev
# Choose a db server (Currently supported: pgsql, mariadb, mysql, mssql, oracle)
export MOODLE_DOCKER_DB=pgsql
# PHP version.
export MOODLE_DOCKER_PHP_VERSION=8.1
# VNC port
export MOODLE_DOCKER_SELENIUM_VNC_PORT=4444
# Browser
export MOODLE_DOCKER_BROWSER=chrome
Then to start the containers for new environment, execute:
bin/moodle-docker-compose down
. ~/docker/moodle-docker.env
bin/moodle-docker-compose up -d
(the disadvantage of this approach is that each env file should override previously loaded env, i.e. they should match in terms of vars they define, but it works for my local dev requirements)
I just done a quick test with a patch:
diff --git a/bin/moodle-docker-compose b/bin/moodle-docker-compose
index 5ae1baf..af04ac9 100755
--- a/bin/moodle-docker-compose
+++ b/bin/moodle-docker-compose
@@ -7,6 +7,8 @@ set -e
thisfile=$( readlink "${BASH_SOURCE[0]}" ) || thisfile="${BASH_SOURCE[0]}"
basedir="$( cd "$( dirname "$thisfile" )/../" && pwd -P )"
+[ ! -f .env ] || export $(grep -v '^#' .env | xargs)
+
if [ ! -d "$MOODLE_DOCKER_WWWROOT" ];
then
echo 'Error: $MOODLE_DOCKER_WWWROOT is not set or not an existing directory'
It actually works and good thing that env vars are runtime specific (not propagated to shell like in my example in the comment above).
To test, apply the patch and execute moodle-docker-compose
from moodle dir (you need to create .env
file also):
moodledev:main> ~/docker/moodle-docker/bin/moodle-docker-compose up -d
It actually works and good thing that env vars are runtime specific
Yeah, I think that to load from env file (ideally dotenv file) is quite possible, like both this and #232 are proposing.
But we need the solution to be:
- Windows supported (note that #232 already had some support for that).
- Check if the order of preference (basically "add only whatever is not already defined") is respected.
With both points fulfilled, I'm happy with any solution, really. I'm also using here some pre-configured "env" files that I source all the time.
Ciao :-)