openfisca / legislation-explorer

Explore legislation formulas and parameters.
https://legislation.demo.openfisca.org
GNU Affero General Public License v3.0
26 stars 12 forks source link

Takes .env into account when building webpack for prod #171

Closed Anna-Livia closed 6 years ago

Anna-Livia commented 6 years ago

When running npm run build, I expect that the environment variables in .env will be taken into account.

However, they are not and the default variables are used instead.

This PR allows users to pul all necessary environement variables in the .envfile

:warning: this has only been tested with the prod environment and not the dev.

MattiSG commented 6 years ago

This says “WIP”, should it be reviewed or not?

fpagnoux commented 6 years ago

~yarn run build && yarn start take the .env file into account on my machine, so I don't think this is an issue with the legislation explorer code itself, but more with the way we run it on the server.~

fpagnoux commented 6 years ago

~I think I got it: to take the .env into account, you need to change the way you start the Legislation explorer.. Adding the --require dotenv/config to the service file should solve the issue.~

fpagnoux commented 6 years ago

Hum, I actually get an error when I try to do that:

openfisca@vps223769:/etc/systemd/system$ /usr/bin/node  --require dotenv/config /home/openfisca/legislation-explorer/index.js
module.js:471
    throw err;
    ^

Error: Cannot find module 'dotenv/config'
    at Function.Module._resolveFilename (module.js:469:15)
    at Function.Module._load (module.js:417:25)
    at Module.require (module.js:497:17)
    at module.js:669:12
    at Array.forEach (native)
    at Function.Module._preloadModules (module.js:668:12)
    at preloadModules (bootstrap_node.js:362:38)
    at startup (bootstrap_node.js:148:9)
    at bootstrap_node.js:504:3

@MattiSG, do you have any experience with using dotenv with systemd, where you cannot use npm start but need to provide absolute path to everything?

fpagnoux commented 6 years ago

~Fixed the previous error by installing dotenv globally (not sure if it's a good practice...)~

fpagnoux commented 6 years ago

~Meh, the command /usr/bin/node --require dotenv/config /home/openfisca/legislation-explorer/index.js dotenv_config_path=/home/openfisca/openfisca-ops/fr.openfisca.org/legislation-explorer/.env works perfectly when running manually, but fails when running within a service 😕 (same error I mentioned before...)~

fpagnoux commented 6 years ago

~Owwkay, found a solution.~

~I'd be in favor of closing this PR, as the initial diagnosis was not accurate: there is no issue at the build, and the .env is taken into account, provided node is given the right arguments.~

~However, it seems that relying on --require dotenv/config is quite systemdunfriendly...~

fpagnoux commented 6 years ago

Okay, I take back most of what I said, and I now understand the issue 🤦‍♂️ . I was just looking at the root page, but indeed loading a variable page doesn't work due to the issue raised by @Anna-Livia.

fpagnoux commented 6 years ago

Reading more about dotenv, https://github.com/openfisca/legislation-explorer/pull/173 seems to be more relevant that the manual processing done here. Closing this PR in favor of #173.