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

Does Configure::load() throw an exception and prevent loading of local file? #5

Open
beporter opened this issue Jun 10, 2015 · 3 comments
Labels

Comments

@beporter
Copy link
Owner

@garethellis36 / @garethellis mentioned to me at CakeFest that there might be an issue with this code snippet :

try {
    $env = getenv('APP_ENV');
    Configure::load("app-{$env}", 'default'); // If this file doesn't exist (say in production)...
    Configure::load('app-local', 'default'); // ...does this still execute?
} catch (\Exception $e) {}

The answer is: I'm not sure-- I don't think I've tested it. If it does, it could be an issue specifically in production situations, where an APP_ENV var isn't meant to even be explicitly required or set. The solution is straightforward though:

try {
    $env = getenv('APP_ENV');
    Configure::load("app-{$env}", 'default');
} catch (\Exception $e) {}
try {
    Configure::load('app-local', 'default');
} catch (\Exception $e) {}

Not quite as succinct, but probably safer. Has anyone tested this? It shouldn't be too difficult using the demo vagrant box in this repo.

@cwbit
Copy link

cwbit commented Jun 10, 2015

Yeah, I end up always doing the second with the two try-catch statements because sometimes (esp on CLI) the ENV isn't set and it causes the whole try/catch to fail, meaning my app-local never gets loaded.

I actually error if the app-local file isn't there because its job is to hold the actual database passwords and other platform-specific settings that can't be in source-control, are critical, and are situation specific. I try and keep as much as possible out of these files but stuff like username/password for database always goes inside my app-local

Here's a snippet from an actual app of mine

/**
 * Load app-env (OPTIONAL)
 *
 * Contains settings that should be overridden at the environment level
 * e.g. setting debug => true for the 'dev' environment but not 'uat'
 *
 * make sure to set in VirtualHost ... SetEnv APP_ENV foo
 */
try {
    $env = getenv('APP_ENV');
    Configure::load("app-{$env}", 'default');
} catch (\Exception $e) {
    # ignore, if not used
}

/**
 * Load app-local (REQUIRED)
 *
 * It is expected that this file will contain, at the min, the database username/password
 */
try {
    Configure::load("app-local", 'default');
} catch (\Exception $e) {
    die($e->getMessage() . "\n");
}

@beporter
Copy link
Owner Author

Yep, easily adaptable to each project's needs. 😄 👍

If I were working with a team, I would probably supplement the die() message to make it more explicit why things failed:

// formatted for readability here
die(
  'Untracked (but mandatory) config/app-local.php missing. '
  . ' Copy & review config/app-local-sample.php to start. Exception message: '
  . $e->getMessage() . "\n"
);

A new dev would only stumble on that exactly once, and also immediately understand how the system worked without any further documentation or "study". Quick recovery plus an environment lesson, in 3 lines of code.

@cwbit
Copy link

cwbit commented Jun 11, 2015

Great idea with smart die() 👍

beporter added a commit that referenced this issue Jun 18, 2015
Addresses the question raised in #5.
beporter added a commit to loadsys/CakePHP-Skeleton that referenced this issue Jun 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants