moodlehq / moodle-docker

A docker environment for moodle developers
GNU General Public License v3.0
373 stars 244 forks source link

Env var to flag moodle-docker environment running #285

Closed kabalin closed 2 months ago

kabalin commented 2 months ago

I am currently using a snippet in Moodle config.php to override my dev configuration when code is tested in moodle-docker:

// Load moodle-docker config file if we are in moodle-docker environment
if (getenv('MOODLE_DOCKER_DBPASS')) {
    require_once($CFG->dirroot . '/config.docker-template.php');
}

require_once($CFG->dirroot . '/lib/setup.php'); // Do not edit.

I am wondering, if it would be beneficial to add a dedicated env var MOODLE_DOCKER that would act as flag (always set to 1) that code is run in moodle-docker for the cases like above? Alternative suggestions (e.g. how you overcome the same problem) are welcome.

stronk7 commented 2 months ago

Heh,

I've a slightly more complex stuff here (I've a minimalistic config.php file that just declares a few array structures (to manage multiple databases and moodledatas) and then includes a configlib.php file that is where everything else happens.

Anyway, blah, blah, blah, that configlib.php is doing EXACTLY the same than your example above, but just using MOODLE_DOCKER_DB instead to decide between "normal" or "moodle-docker" configurations.

About the suggestion regarding the creation and export of a new MOODLE_DOCKER env variable (meaning, "hey, I'm running a moodle-docker environment") , I don't oppose although, in the other side, it doesn't provide much, given we already have some exclusive/required env variables that can be checked.

So, my side, I'm neutral here. Acceptable.

Ciao :-)

kabalin commented 2 months ago

I don't oppose although, in the other side, it doesn't provide much, given we already have some exclusive/required env variables that can be checked

Thinking in the direction if it can provide something meaningful:

diff --git a/base.yml b/base.yml
index 28414bd..2ca95a9 100644
--- a/base.yml
+++ b/base.yml
@@ -7,6 +7,7 @@ services:
       - "${MOODLE_DOCKER_WWWROOT}:/var/www/html"
       - "${ASSETDIR}/web/apache2_faildumps.conf:/etc/apache2/conf-enabled/apache2_faildumps.conf"
     environment:
+      MOODLE_DOCKER_PROJECT_NAME: "${COMPOSE_PROJECT_NAME}"
       MOODLE_DOCKER_DBNAME: moodle

which can be of use as both flag "hey, I'm running a moodle-docker environment" and some potential to do something in code, based on the project instance... although we should not forget about YAGNI

stronk7 commented 2 months ago

Problem with that one is that it's empty by default (unless you're setting up COMPOSE_PROJECT_NAME), hence it won't work for any "anonymous" instance. Also, why would you want to "duplicate" something that already exists, I mean, the second potential use proposed ("to do something in code") already can be done using the compose original env var, isn't it?

I can foresee 2 alternatives (env var name apart, just the concept):

Ciao :-)

kabalin commented 2 months ago

On my "anonymous" instance COMPOSE_PROJECT_NAME is set to moodle-docker... It does not propagates to container itself (there is no COMPOSE_PROJECT_NAME in container env), but MOODLE_DOCKER_PROJECT_NAME=moodle-docker is set (when run with that patch above).

But you are right re duplication.

paulholden commented 2 months ago

The docs added as part of this issue are not correct, in fact if used verbatim they will cause a fatal error on a site using standard config.php file:

[Fri Apr 19 13:27:27.545671 2024] [php:warn] [pid 59328] [client 192.168.123.1:55168] PHP Warning:  Undefined property: stdClass::$dirroot in /opt/moodle/master/src/config.php on line 55, referer: http://moodle.internal/master/my/
[Fri Apr 19 13:27:27.546109 2024] [php:warn] [pid 59328] [client 192.168.123.1:55168] PHP Warning:  require_once(/lib/setup.php): Failed to open stream: No such file or directory in /opt/moodle/master/src/config.php on line 55, referer: http://moodle.internal/master/my/
[Fri Apr 19 13:27:27.546365 2024] [php:error] [pid 59328] [client 192.168.123.1:55168] PHP Fatal error:  Uncaught Error: Failed opening required '/lib/setup.php' (include_path='.:/usr/share/php') in /opt/moodle/master/src/config.php:55\nStack trace:\n#0 /opt/moodle/master/src/login/logout.php(27): require_once()\n#1 {main}\n  thrown in /opt/moodle/master/src/config.php on line 55, referer: http://moodle.internal/master/my/

https://github.com/moodlehq/moodle-docker/pull/289 for your consideration