Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Env var to flag moodle-docker environment running #285

Closed
kabalin opened this issue Apr 7, 2024 · 5 comments · Fixed by #288
Closed

Env var to flag moodle-docker environment running #285

kabalin opened this issue Apr 7, 2024 · 5 comments · Fixed by #288

Comments

@kabalin
Copy link
Member

kabalin commented Apr 7, 2024

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
Copy link
Member

stronk7 commented Apr 8, 2024

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
Copy link
Member Author

kabalin commented Apr 8, 2024

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
Copy link
Member

stronk7 commented Apr 8, 2024

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):

  • A simple: MOODLE_DOCKER_RUNNING: 1 (and done)
  • A more complex MOODLE_DOCKER_RUNNING: "${COMPOSE_PROJECT_NAME:-1}" (not tested, but I think it works)

Ciao :-)

@kabalin
Copy link
Member Author

kabalin commented Apr 9, 2024

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.

kabalin added a commit to kabalin/moodle-docker that referenced this issue Apr 9, 2024
…r run.

MOODLE_DOCKER_RUNNING=1 is available inside webserver continer,
this can be used to conditionally load test configuration.

Closes moodlehq#285
paulholden added a commit to paulholden/moodle-docker that referenced this issue Apr 19, 2024
@paulholden
Copy link
Member

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/

#289 for your consideration

skodak pushed a commit to skodak/moodle-docker that referenced this issue Aug 20, 2024
…r run.

MOODLE_DOCKER_RUNNING=1 is available inside webserver continer,
this can be used to conditionally load test configuration.

Closes moodlehq#285

# Conflicts:
#	base.yml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants