Use .env file for directory/db #130

iwasherefirst2 · 2020-06-23T22:58:12Z

Using an environment file for directory/db
has the advantage that the user does not need
to specify the variables after each reboot.

Added .env to gitignore list, to avoid
that environment variables end up in repository.

Created moodle-start for convenience.

Updated readme.

Closes #80
Closes #81

Using an environment file for directory/db has the advantage that the user does not need to specify the variables after each reboot. Added `.env` to gitignore list, to avoid that envirenment variables end up in repository. Created `moodle-start` for convenience. Imroved new readme. Closes #80 Closes #81

danpoltawski · 2020-06-24T00:04:25Z

README.md

-export MOODLE_DOCKER_DB=pgsql
-
-# Ensure customized config.php for the Docker containers is in place
-cp config.docker-template.php $MOODLE_DOCKER_WWWROOT/config.php


This still seems necessary for someone to quick start?

Whoups your right. I added a create-moodle-config script and updated the readme in the latest commit.

danpoltawski · 2020-06-24T00:17:05Z

bin/moodle-start

+#!/usr/bin/env bash
+
+bin/moodle-docker-compose up -d
+bin/moodle-docker-wait-for-db


I'm not very convinced of the value of this moodle-start script. My 'philosophy' when originally designing this project was just to add the 'minimum helpers' to get by for convenience and scriptability, but encourage people to use the docker compose commands when necessary. This seems to abstracting what's going on under the hood more than I think this project should. For most people, doing moodle-docker-compose up -d will be enough for their workflow of development.

I don't think this script is well named.

I understand your philosophy and removed the script.

danpoltawski · 2020-06-24T00:23:12Z

I realise I just posted two negative comments - I think the rest of the .env support is a great idea - thank you for contributing it. :-) 👍 - just wanted to make my thinking clear, @moodlehq now maintain this project.

iwasherefirst2 · 2020-06-24T05:51:18Z

I realise I just posted two negative comments - I think the rest of the .env support is a great idea - thank you for contributing it. :-) +1 - just wanted to make my thinking clear, @moodlehq now maintain this project.

Thank you. I also think using an .env is very convenient. Writing the path to your Moodle in the terminal every time after I boot my computer was time-consuming (long path) and error prone (typo). Also when I read the readme for the first time, I was very confused about the Quick start section. I was sure that this was a script that I have to adjust, so I went to all files in the folder. I was surprised when I found out that I actually have to type all this in the terminal. That is why I tried to be more explicit in the new readme.

Let people decide if they want to set the directory path in the terminal instead of the envirenment file. This also should make travis pass the tests.

stronk7 · 2021-05-30T22:13:29Z

Hi @iwasherefirst2,

nice improvement!

Would you mind, please, rebasing your commits on top of current master (I think you will face some minor conflicts to solve with that). And then proceed to squash current 6 commits, so everything can get the final review and, hopefully, land all-together?

Ciao :-)

stronk7 · 2021-05-30T22:27:00Z

Side comment about this patch, because for example it will affect some of my usual flows with moodle-docker.

Maybe the .env file should be setting the env variables only if they aren't defined ? I mean, basically act as "defaults" more that "environment" and only apply for them if undefined.

Real example: I've here some wrappers over moodle-docker that are basically a couple of nested loops changing the PHP versions (MOODLE_DOCKER_PHP_VERSION ) and the databases (MOODLE_DOCKER_DB). I know that I can continue using it as is, as far as I don't create the .env file... everything will continue working the same.

But, if I normally want to work with postgres (for example) and I'm interested into the new file... then the wrapper above will stop working, because the variables in that file will, always, get precedence over the already specified ones.

So, just guessing if we should consider "renaming" all this from "env" to "defaults" and provide that slightly different behavior. For people not switching often it will behave 100% the same. But for crazy-switchers will make life easier.

Thoughts?

BTW, another little idea, is that maybe the .env.example template, better if it's not invisible and, also, maybe better if it has some comment about the possibility of setting up any other env variable there to make it explicitly clear.

Up to you, only a couple of details that came to my brain when reviewing current proposed patch.

Thanks for your contribution, much appreciated!

Ciao :-)

septatrix

I do not think this add any value (besides the addition to the gitignore file).
If people want to save these variables they are free to create a .env file themselves and source it beforehand. This is also what every seasoned dev probably already does. Therefore adding the entry to the gitignore file seems like a good idea and mentioning that this can be done in the docs will not hurt either. However adjusting all the scripts etc does not really add any value IMO.

septatrix · 2021-07-02T15:02:05Z

bin/create-moodle-config

+
+if [ -f .env ]; 
+then
+    source .env


This will not work as the variables are not exported (at least inside you sample env file)
Also applies to all following uses of source .env

septatrix · 2021-07-02T15:04:16Z

README.md

+4. Run `bin/create-moodle-config`. This copies the `config.php` into Moodle.
+You only have to call this command onces.
+4. Run `bin/moodle-docker-compose up -d` inside `moodle-docker` folder to create the container.
+5. You may have to call also `bin/moodle-docker-wait-for-db` if you use `oracle/mssql`.


You may have to -> You should (at least to my knowledge you always want to do this if you execute these commands directly after each other)

septatrix · 2021-07-02T15:04:50Z

README.md

+2. Rename `.env.example` to `.env`
+3. Specify Moodle directory and db driver in `.env`
+4. Run `bin/create-moodle-config`. This copies the `config.php` into Moodle.
+You only have to call this command onces.


This seems to me like a more complicated setup than what was here previously.

septatrix · 2021-07-02T15:06:04Z

.env.example

+# Set up path to Moodle code
+MOODLE_DOCKER_WWWROOT=/path/to/moodle/code
+# Choose a db server (Currently supported: pgsql, mariadb, mysql, mssql, oracle)
+MOODLE_DOCKER_DB=pgsql


These need an export before them if you want to source them from the scripts. dotenv files like you know them from php, python etc will not work with shells.

skodak · 2022-10-07T10:39:16Z

I have used a bit different approach in #232

danpoltawski reviewed Jun 24, 2020

View reviewed changes

iwasherefirst2 added 3 commits June 24, 2020 07:29

Remove export key from env-example

20c16cd

Add create-moodle-config script

f7dad4f

Remove unprecise-named script

200981f

iwasherefirst2 added 2 commits June 24, 2020 07:59

Check if .env file exists before sourcing it

5aa57ae

Don't throw error if env is missing

25750bf

Let people decide if they want to set the directory path in the terminal instead of the envirenment file. This also should make travis pass the tests.

jpahullo mentioned this pull request Feb 7, 2021

add suport for .env files #81

Closed

septatrix suggested changes Jul 2, 2021

View reviewed changes

iwasherefirst2 closed this by deleting the head repository Jul 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use .env file for directory/db #130

Use .env file for directory/db #130

iwasherefirst2 commented Jun 23, 2020

danpoltawski Jun 24, 2020

iwasherefirst2 Jun 24, 2020

danpoltawski Jun 24, 2020

iwasherefirst2 Jun 24, 2020

danpoltawski commented Jun 24, 2020

iwasherefirst2 commented Jun 24, 2020

stronk7 commented May 30, 2021

stronk7 commented May 30, 2021

septatrix left a comment

septatrix Jul 2, 2021

septatrix Jul 2, 2021

septatrix Jul 2, 2021

septatrix Jul 2, 2021

skodak commented Oct 7, 2022

Use .env file for directory/db #130

Use .env file for directory/db #130

Conversation

iwasherefirst2 commented Jun 23, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danpoltawski commented Jun 24, 2020

iwasherefirst2 commented Jun 24, 2020

stronk7 commented May 30, 2021

stronk7 commented May 30, 2021

septatrix left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skodak commented Oct 7, 2022