Skip to content

Populate dotenv to config#409

Closed
BernardGoldberger wants to merge 7 commits intokoel:masterfrom
BernardGoldberger:moveENVtoConfig
Closed

Populate dotenv to config#409
BernardGoldberger wants to merge 7 commits intokoel:masterfrom
BernardGoldberger:moveENVtoConfig

Conversation

@BernardGoldberger
Copy link
Copy Markdown
Contributor

I noticed that occasionally I would have an error

[2016-08-10 00:23:08] production.ERROR: PDOException: SQLSTATE[HY000] [1045] Access denied for user 'forge'@'localhost' (using password: NO) in C:\wamp64\www\koel\vendor\laravel\framework\src\Illuminate\Database\Connectors\Connector.php:55

of course this make no sense since I have set the DB and password.

So I did some research and found 2 things

  1. dotenv should only be used in development, once in production it should all be cached.
  2. dotenv should only be populated in the config directory.

Here is a post vlucas/phpdotenv#76 (comment) by @GrahamCampbell about this.

In this PR I created a separate config file koel.php for all Koel specific configurations, and I also fix #408.

Going forward we can tell users to run php artisan config:cache once they finnished setting up Koel, or we could even make part of the koel:init command.

Note

I think this might in fact be the actual issue with #393.

Comment thread database/seeds/UserTableSeeder.php Outdated
if (!env('ADMIN_NAME') || !env('ADMIN_EMAIL') || !env('ADMIN_PASSWORD')) {
if (!config('koel.admin.name') ||
!config('koel.admin.email') ||
!config('koel.admin.password'))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected 1 space after closing parenthesis; found 9

Comment thread database/seeds/UserTableSeeder.php Outdated
if (app()->environment() !== 'testing') {
$this->command->info('Admin user created. You can (and should) remove the auth details from .env now.');
if (! app()->runningUnitTests()) {
$this->command->info('Admin user created. You can (and should) remove the auth details from .env now.');
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line indented incorrectly; expected at least 12 spaces, found 8

@BernardGoldberger
Copy link
Copy Markdown
Contributor Author

I will fix it all up.

Comment thread config/koel.php
'admin' => [
'name' => env('ADMIN_NAME'),
'email' => env('ADMIN_EMAIL'),
'password' => env('ADMIN_PASSWORD'),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code style.

@phanan
Copy link
Copy Markdown
Member

phanan commented Aug 17, 2016

This looks great! And thank you for your findings – I learned something new today. Can you

  • Resolve the conflicts
  • Squash the commits into one
  • Separate the seeder into another PR?

@BernardGoldberger
Copy link
Copy Markdown
Contributor Author

I'm not that good at this :-(

Close this PR and I will make them from scratch.

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

Successfully merging this pull request may close these issues.

Bug: artisan config:cache

3 participants