moodle-docker icon indicating copy to clipboard operation
moodle-docker copied to clipboard

Add support for ./moodle-docker.env

Open skodak opened this issue 2 years ago • 15 comments

See the README.md for more information how to use it.

My main objectives were:

  • do not require setting MOODLE_DOCKER_WWWROOT when executing compose from Moodle checkout dir
  • do not require COMPOSE_PROJECT_NAME when having multiple Moodle checkouts dir
  • allow the db default to be stored in some file in Moodle checkout dir

NOTE: this feature is not supported in Windows because I do not know how to parse the env file there.

skodak avatar Oct 07 '22 09:10 skodak

Hi @skodak,

According to docker-compose documentation, .env are also loaded in a prioritized way of loading variables. This is why I suggested add support for .env on #80. Natively it would be supported by Windows platforms.

However, it is a big and final step towards supporting several Moodle versions locally. Super! Congrats!

jpahullo avatar Oct 07 '22 10:10 jpahullo

hi @jpahullo , docker .env loading is not applicable in moodle-docker-compose script because we need the environment variables BEFORE the real docker-compose is executed to create configuration for it.

moodle-docker.env file contains environment settings for moodle-docker-compose script (and for docker too).

With my patch you can still use .env, but the moodle-docker-compose script cannot use values from it.

I think I am onto something with the Windows support, I might try to make it work this weekend unless somebody volunteers.

skodak avatar Oct 07 '22 11:10 skodak

Hi @skodak ! You're totally right! I missed this part when commenting.

Thanks!!!

jpahullo avatar Oct 07 '22 11:10 jpahullo

hehe, I would not know if I did not try to create .env and failing badly...

skodak avatar Oct 07 '22 11:10 skodak

I should have Windows support for this issue later today

skodak avatar Oct 10 '22 12:10 skodak

argh, not there yet, I ned to find out how to check if %%1 env variable is set

skodak avatar Oct 10 '22 13:10 skodak

now it should be finally ready for review with full Windows support, please note the "setlocal" command which makes the env variables behave more like in bash, without it the SET would propagate into the next execution and break stuff

skodak avatar Oct 10 '22 14:10 skodak

(sorry for the delay, @skodak, will be looking to this ASAP!)

stronk7 avatar Oct 13 '22 10:10 stronk7

any progress @stronk7 ?

skodak avatar Oct 20 '22 09:10 skodak

Any news from this?

Thanks a lot for your work!

jpahullo avatar Mar 17 '23 21:03 jpahullo

See also #249 which uses shdotenv to get a better support for dotenv files.

andrewnicols avatar Mar 18 '23 05:03 andrewnicols

Hi,

is there any reason we cannot make the env file to be a truly "dotenv" (.env) one?

Also is the "define only if not set" fallback respected?

  1. Env variables already defined via export/set.
  2. Env files support introduced by this issue.
  3. moodle-docker own defaults.

Note I'm also looking to #249, really both are similar (add to the environment some variables, if they are not defined), using 2 different approaches.

I like here that it supports (not tested) Windows and that there aren't external dependencies. In the other side, surely the supported variables will be a little more limited, but I also think that it will be enough for what moodle-docker needs (simple 1-word contents).

Ciao :-)

stronk7 avatar Mar 21 '23 08:03 stronk7

Hi,

is there any reason we cannot make the env file to be a truly "dotenv" (.env) one?

Also is the "define only if not set" fallback respected?

  1. Env variables already defined via export/set.
  2. Env files support introduced by this issue.
  3. moodle-docker own defaults.

Note I'm also looking to #249, really both are similar (add to the environment some variables, if they are not defined), using 2 different approaches.

I like here that it supports (not tested) Windows and that there aren't external dependencies. In the other side, surely the supported variables will be a little more limited, but I also think that it will be enough for what moodle-docker needs (simple 1-word contents).

Ciao :-)

+1

jpahullo avatar Mar 21 '23 09:03 jpahullo

Hi @skodak , Thanks for working on this. I’ve given it a review. In addition to @stronk7 comment about confirming the fallback precedence: The main change required here is to include the code in include/env.sh into the moodle-docker-compose. Including it via source is an unnecessary abstraction in this case.

Then (taking a couple of pointers from #249):

  • Make an .env.example file and commit that
  • Update the readme to refer to using and renaming the .env.example file
  • Update the script that parses the file to expect .env as the file name
  • Add .env to .gitignore so no one accidentally ever raises a PR with their credentials in it.

Cheers, Matt P

mattporritt avatar Mar 22 '23 05:03 mattporritt

sorry @mattporritt , but the include/env.sh is essential when you need to use same environment in another shell script, such as waiting for moodle app

in any case I have decided to maintain my own fork because I need this for all my development, so I will not be submitting any more pull requests here

skodak avatar Apr 24 '23 09:04 skodak