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

Make file loading optional #63

Closed
sagikazarmark opened this issue Jan 18, 2015 · 19 comments
Closed

Make file loading optional #63

sagikazarmark opened this issue Jan 18, 2015 · 19 comments

Comments

@sagikazarmark
Copy link

First time I use this, so first of all: thanks for the awesome library!

Secondly: I was thinking about if it is possible to make file loading optional. Let me explain with an example.

I am currently working on a few CLI tools where third party APIs are invlolved. In most cases the app needs an API key. To make this possible the user have a few possibilities to choose from:

  • Create a config.yaml in the working directory
  • Create a .config.yaml in the home directory
  • Pass a --key option along with the API key

In this case it is clear that only one of them is required.

Now let's change this to an environment variable based solution:

  • Create a .env file in the working directory
  • Add an API_KEY variable to your .bashrc, etc
  • Run the command with API_KEY variable included

But in this case the first one is required now, since Dotenv::load() (or $dotenv->load() in the new version) requires that file to be there.

Is there any solution to make it optional? Load it if exists. Since the check for required variables depends on ALL existing variable, not only the ones loaded from the file, this could be an easy change.

The obvious (but IMO ugly) solution would be to catch any thrown exception, if the file is not found.

What do you think?

@sagikazarmark
Copy link
Author

Also, related to the CLI context: wouldn't it be better to use getcwd() instead of __DIR__?

@vlucas
Copy link
Owner

vlucas commented Jan 19, 2015

Step 2 in your new flow with dotenv can be changed to running a single command:

source .env

@sagikazarmark
Copy link
Author

Of course it can be, that was just an example of setting up variables in our environment. Thanks anyway. :)

@sagikazarmark
Copy link
Author

@vlucas Anything to add here?

@vlucas
Copy link
Owner

vlucas commented Jan 30, 2015

I think Dotenv is always going to expect the file to exist. You could either try/catch, or just check it yourself:

if (file_exists($envDir . '.env')) {
   $dotenv->load($envDir);
}

@vlucas
Copy link
Owner

vlucas commented Jan 30, 2015

Of course, the other solution might be something like:

$dotenv->loadIfExists();

@sagikazarmark
Copy link
Author

That's what I was hoping ;)

@vlucas vlucas closed this as completed Jan 30, 2015
@vlucas vlucas reopened this Jan 30, 2015
@kelvinj
Copy link
Contributor

kelvinj commented Feb 1, 2015

$dotenv->loadIfExists();

and then

$dotenv->overloadIfExists();

feels like overkill.

I'd rather check the file exists or wrap it up in a try … catch as Vance said.

@mikehaertl
Copy link

@vlucas Maybe I have a big misunderstanding here (I'm new to dotenv) but I was very very surprised that this .env file is mandatory. I thougth, that's the point of dotenv:

  • Use a .env file in your local environment
  • Use "real" env vars in production (no .env required there!)

I find it very weird, that there's an extra check required in my code, whether .env exists. I think, the library should handle this transparently.

Or am I completely missing the point here?

@vlucas
Copy link
Owner

vlucas commented Feb 17, 2015

You generally should only be using Dotenv for development and/or testing, e.g.:

if(getenv('APP_ENV') === 'development') {
    $dotenv = new Dotenv\Dotenv();
    $dotenv->load(__DIR__);
}

This is why the .env is generally always expected to be present.

@mikehaertl
Copy link

Hmm, but then you have a chicken-egg problem: In dev mode you would use Dotenv to set env vars, so you can't set APP_ENV in the first place.

I really can't see the drawback if Dotenv would be more relaxed in regard to .env: If it's there, load it. If not - ignore anything Dotenv related.

Of course I can add a if (file_exists('.env')) - it's just some extra inconvenience. Maybe I see something in Dotenv that it doesn't want to be: A transparent drop-in replacement for environment vars. It should only do something if .env exists - and stay out of the way if no such file is present.

@kelvinj
Copy link
Contributor

kelvinj commented Feb 21, 2015

Despite Vance being quite resolute in saying that this should not be used in production, that is a subjective viewpoint that has no bearing on how people actually use dotenv.

I think you have 2 camps:

  1. People who use it in development to simulate production environment variables
  2. People who use it in all environments to put their app in a known state so they don't have to rely on external environment variables.

The first camp, right now, has to check for the file as it's optional. However if you were to make the suggested change you would just be forcing the second camp to do the check to ensure the file is present.

Such a pointless change and one you could argue over until the cows come home.

@sagikazarmark
Copy link
Author

I agree with @vlucas: using dotenv in production SHOULD be avoided if POSSIBLE. The extra file loading is overhead. That said, it is just not possible in some cases.

However it doesn't really matter for example in case of a console application.

@sagikazarmark
Copy link
Author

@vlucas related to the recommended usage, I just realised something: Dotenv is not just able to load AND validate env variables, it can do valdation on it's own.

So here is how I would modify the "official" recommendation (as an option, since validation is not necessary):

$dotenv = new Dotenv\Dotenv();
if(getenv('APP_ENV') === 'development') {
    $dotenv->load(__DIR__);
}
$dotenv->required('OTHER_VAR');

This allows us to skip file loading, but not the validation.

Does that make sense?

@vlucas
Copy link
Owner

vlucas commented Apr 4, 2015

@sagikazarmark that makes perfect sense. I do exactly that. In fact, this is real code I just copied from a live application of mine running on bullet:

define('BULLET_ENV', $request->env('BULLET_ENV', 'development'));

// ...

// Load required environment variables from .env in development
$dotenv = new Dotenv\Dotenv();
if(BULLET_ENV == 'development') {
    $dotenv->load(dirname(__DIR__), 'dev.env');
}
// Required ENV vars for this app in production
$dotenv->required([
    'DATABASE_URL',
    'PATH_FILE_UPLOADS',
    'GOOGLE_PLACES_API_KEY',
    'RAVEN_DSN',
    'MANDRILL_USER', 'MANDRILL_PASS'
]);

@koenpunt
Copy link

koenpunt commented Apr 5, 2015

I've created a PHP extension to handle .env files in production. Its still very beta, and could use some testers, so if anyone wants to give it a go: https://github.com/koenpunt/php-dotenv

@vlucas
Copy link
Owner

vlucas commented May 30, 2015

I think this has been sufficiently answered, so I am closing this for now.

@vlucas vlucas closed this as completed May 30, 2015
@armetiz
Copy link

armetiz commented Jun 23, 2015

Hi there,

On master branch (and all latest branches & 2.0.1), the Dotenv constructor should take directory and file name.

Using a conditional to use load or not seems weird.

$dotenv = new Dotenv\Dotenv(dirname(__DIR__) . '/etc', 'config.env');

if(false === $isProductionEnvironment) {
    $dotenv->load();
}

$dotenv->required(['FOO']);

Should constructor be updated to avoid parameters ?

Regards

@madsem
Copy link

madsem commented Jun 7, 2018

@mikehaertl this is exactly the problem I was just trying to wrap my head around.
(Also completely new to dotenv)

And this is still actual, so I assume it's okay to comment in this old issue.

The way I solved it now was to just reverse the statement and check if APP_ENV is not in production.
i.e:

/*
 * PHP DotEnv initialization
 *
 * Production expects to have everything in the .env exported,
 * as actual environment variables on the server(s)
 */
if(getenv('APP_ENV') !== 'production') {
    $dotEnv = new Dotenv(dirname(__DIR__));
    $dotEnv->load();
}

Not quite sure if this is the recommended way of using it @vlucas ?
Sorry if I'm asking something obvious, it just really is unclear to me right now and it seems also to others, at least judging by what I read over the last 2 hours Googling this :D.

PS: Doesn't that also mean that dotenv should be installed with the dev flag for composer?!

composer require --dev vlucas/phpdotenv

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants