[FrameworkBundle] Expose dotenv in bin/console about #24144

ro0NL · 2017-09-10T13:17:28Z

Q	A
Branch?	3.4
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes/no
Fixed tickets	#...
License	MIT
Doc PR	symfony/symfony-docs#...

For completeness in CLI, shown in web under "Server parameters".

(new Dotenv())->populate(['APP_ENV' => 'prod', 'APP_DEBUG' => '1']);

ro0NL · 2017-09-10T14:42:49Z

While at it patched the command to use getContainer()->getParamater('kernel.X_dir') in favor of getXDir() directly. Follows consensus reached in #23624 and applies to 3.4.

javiereguiluz · 2017-09-11T06:58:53Z

src/Symfony/Bundle/FrameworkBundle/Command/AboutCommand.php

+        if ($dotenv = self::getDotEnvVars()) {
+            $rows = array_merge($rows, array(
+                new TableSeparator(),
+                array('<info>Dotenv</>'),


Dotenv doesn't mean much unless you know what is Dotenv. Could we rename this to Environment variables loaded by Symfony or something like that? Thanks.

Fair point. Let me think of better name, yet i could live with "Dotenv" :) more suggestions welcome.

What about Environment variables (dotenv)?

It's enabled by using Dotenv directly, so should be clear.

I'd use Environment variables (.env) which seems more "correct". The user defined the vars in a .env file and they don't care about how they are loaded.

Managed environment variables could do

javiereguiluz · 2017-09-12T15:30:13Z

I'm still not convinced by the naming of this option. Here we say "Environment variables (.env)" but in #23951 we say "Loaded variables".

The naming must be perfectly precise: these are only the env vars loaded by Symfony from your local .env file. They are not all the env vars defined or available in your app.

fabpot · 2017-09-12T15:52:52Z

Somehow related: symfony/recipes#176

stof · 2017-09-12T16:12:41Z

src/Symfony/Bundle/FrameworkBundle/Command/AboutCommand.php

+    private static function getDotEnvVars()
+    {
+        $vars = array();
+        foreach (explode(',', getenv('SYMFONY_DOTENV_VARS')) as $name) {


this will break when getenv returns false

@stof, explode returns an array with one empty string:

>>> explode(',', false) => [ "", ] >>>

ro0NL · 2017-09-12T17:27:44Z

Here we say "Environment variables (.env)" but in #23951 we say "Loaded variables".

I think it should follow with Environment vars (dotenv) or whatever we decide here (or there :)). They should be the same, yes.

From symfony/recipes#176 (comment)

What we could do instead is having a CLI tool that list defined env vars and tell the dev where they are coming from (env var, .env, $_SERVER, ...).

Im worried for a big list of envs, cluttering the output. IMHO about only cares about the SF ones. System-wide just do php -r "var_dump($_ENV, $_SERVER);" no?

Same applies for web. System-wide you can check phpinfo or so (available via the profiler already). Yet we decided to list all server parameters (thus including those loaded by dotenv). Space is less an issue here.

To mimic "loaded by dotenv" behavior in web, i suggest to highlight those in the current server parameters table and avoid extra panels etc.. See #23951

And technically these variables are not loaded/set by SF persee, only managed.

ro0NL · 2017-09-12T19:55:23Z

@fabpot @javiereguiluz i just realized.. there is bin/console about --help also. Great place for discovery/explanation. That could justify using Dotenv again by explaining what that means with --help.

I like the simplicity it brings for just describing the feature we're implementing; expose 1) Symfony 2) Dotenv 3) Variables, a.k.a. 1)SYMFONY_2)DOTENV_3)VARS.

For this PR that is

1 = about
2 = Dotenv table header
3 = the table is clearly a key/value table

We shouldnt hide this is about Dotenv IMHO.

ro0NL · 2017-09-12T20:33:54Z

Updated. Latest version;

$ bin/console about

 -------------------- --------------------------------- 
  Environment (.env)                                    
 -------------------- --------------------------------- 
  APP_ENV              prod                             
  APP_DEBUG            1                                
 -------------------- --------------------------------- 

$ bin/console about --help

Help:
  The about command displays information about the current Symfony project.
  
  The PHP section displays configuration that might differ between web and CLI.
  
  The Environment section displays the current environment variables managed by Symfony Dotenv. It will not
  be shown if no variables were found.

Works for me :)

yceruto · 2017-09-22T11:15:50Z

src/Symfony/Bundle/FrameworkBundle/Command/AboutCommand.php

@@ -57,14 +71,12 @@ protected function execute(InputInterface $input, OutputInterface $output)
            new TableSeparator(),
            array('<info>Kernel</>'),
            new TableSeparator(),
-            array('Type', get_class($kernel)),
-            array('Name', $kernel->getName()),


Please, don't remove the kernel name info, it is important for Name-based Virtual Kernel 4.0 applications :(

edit: I means, it is the way I can to know which app is running according to the kernel name.

Agreed: please don't remove this output. Regardless, even if there was merit to the change, this seems outside the scope of this PR. We should avoid surprises that aren't directly related to the PR itself; otherwise, people will have difficulty even finding the discussion around the change.

Well if we get rid of Kernel::getName() then we're good anyway :)

Maybe i can look into that soonish; symfony/recipes#186 (comment)

In general i agree; if we keep it, it should be displayed here. Tend to remove Kernel type either way, you probably "know" this.

Reverted for now :) should be discussed when deprecating getName.

Let's not introduce something we know will be deprecated. If you really want to display something here, we could use the Kernel class name. That would help differentiate multi-kernel apps.

We already show the kernel class name, no? Regardless, the issue isn't with introducing something, but that this PR removed something unrelated to the stated purpose of this PR.

IMHO, the call to getName() here should be discussed (and possibly removed) when a PR to remove the Kernel::getName() method is introduced, not in a PR adding Dotenv output. Right?

ro0NL · 2017-09-26T15:21:21Z

Perhaps we should consider a special command for this as well. Tend to like debug:env.

See #22406 (comment)

But, the current proposed feature, works for me 👍 although not used in real life projects with many env e.g.

fabpot · 2017-09-30T14:35:16Z

src/Symfony/Bundle/FrameworkBundle/Command/AboutCommand.php

-            array('Log directory', self::formatPath($kernel->getLogDir(), $kernel->getProjectDir()).' (<comment>'.self::formatFileSize($kernel->getLogDir()).'</>)'),
+            array('Root directory', self::formatPath($container->getParameter('kernel.root_dir'), $projectDir)),
+            array('Cache directory', self::formatPath($cacheDir = $container->getParameter('kernel.cache_dir'), $projectDir).' (<comment>'.self::formatFileSize($cacheDir).'</>)'),
+            array('Log directory', self::formatPath($logDir = $container->getParameter('kernel.logs_dir'), $projectDir).' (<comment>'.self::formatFileSize($logDir).'</>)'),


Why is it better to use parameters?

nothing really, it's just that getProjectDir is not part of KernelInterface, and $kernel->getContainer()->getParameter('kernel.some_path') works for all paths / kernel instances.

See also #23624 (comment) where we favored getParam over injection.

I don't like it. I don't buy the "it is not part of KernelInterface". Using a string sounds much worse than using a method.

fabpot · 2017-10-03T18:11:42Z

Thank you @ro0NL.

…ro0NL) This PR was merged into the 3.4 branch. Discussion ---------- [FrameworkBundle] Expose dotenv in bin/console about | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes/no | Fixed tickets | #...  | License | MIT | Doc PR | symfony/symfony-docs#...  For completeness in CLI, shown in web under "Server parameters". ```php (new Dotenv())->populate(['APP_ENV' => 'prod', 'APP_DEBUG' => '1']); ``` ![image](https://user-images.githubusercontent.com/1047696/30293203-fe360d0a-9738-11e7-80ed-94901cea8898.png) Commits ------- 9011f47 [FrameworkBundle] Expose dotenv in bin/console about

carsonbot added Status: Needs Review FrameworkBundle Feature labels Sep 10, 2017

ro0NL mentioned this pull request Sep 10, 2017

[WebProfilerBundle] Add a pane with loaded envs #23951

Closed

nicolas-grekas added this to the 3.4 milestone Sep 11, 2017

javiereguiluz reviewed Sep 11, 2017

View reviewed changes

fabpot mentioned this pull request Sep 12, 2017

checking for .env file exists instead of using the env variable symfony/recipes#176

Closed

stof requested changes Sep 12, 2017

View reviewed changes

ro0NL mentioned this pull request Sep 22, 2017

Fix kernel name symfony/recipes#186

Closed

yceruto reviewed Sep 22, 2017

View reviewed changes

stof approved these changes Sep 27, 2017

View reviewed changes

fabpot reviewed Sep 30, 2017

View reviewed changes

[FrameworkBundle] Expose dotenv in bin/console about

9011f47

fabpot approved these changes Oct 3, 2017

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Oct 3, 2017

fabpot merged commit 9011f47 into symfony:3.4 Oct 3, 2017

ro0NL deleted the about-env branch October 3, 2017 18:31

This was referenced Oct 18, 2017

Release v3.4.0-BETA1 #24609

Merged

Release v4.0.0-BETA1 #24612

Merged

ro0NL mentioned this pull request Nov 26, 2017

[WebProfilerBundle] Expose dotenv variables #25166

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FrameworkBundle] Expose dotenv in bin/console about #24144

[FrameworkBundle] Expose dotenv in bin/console about #24144

ro0NL commented Sep 10, 2017 •

edited

Loading

ro0NL commented Sep 10, 2017

javiereguiluz Sep 11, 2017

ro0NL Sep 11, 2017

ro0NL Sep 11, 2017 •

edited

Loading

fabpot Sep 11, 2017

ro0NL Sep 11, 2017 •

edited

Loading

javiereguiluz commented Sep 12, 2017

fabpot commented Sep 12, 2017

stof Sep 12, 2017

voronkovich Sep 12, 2017

ro0NL commented Sep 12, 2017 •

edited

Loading

ro0NL commented Sep 12, 2017 •

edited

Loading

ro0NL commented Sep 12, 2017

yceruto Sep 22, 2017 •

edited

Loading

robfrawley Sep 22, 2017

ro0NL Sep 22, 2017

ro0NL Sep 22, 2017

fabpot Sep 22, 2017

robfrawley Sep 22, 2017

ro0NL commented Sep 26, 2017

fabpot Sep 30, 2017

ro0NL Oct 2, 2017 •

edited

Loading

fabpot Oct 2, 2017

ro0NL Oct 3, 2017

fabpot commented Oct 3, 2017

[FrameworkBundle] Expose dotenv in bin/console about #24144

[FrameworkBundle] Expose dotenv in bin/console about #24144

Conversation

ro0NL commented Sep 10, 2017 • edited Loading

ro0NL commented Sep 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ro0NL Sep 11, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ro0NL Sep 11, 2017 • edited Loading

Choose a reason for hiding this comment

javiereguiluz commented Sep 12, 2017

fabpot commented Sep 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ro0NL commented Sep 12, 2017 • edited Loading

ro0NL commented Sep 12, 2017 • edited Loading

ro0NL commented Sep 12, 2017

yceruto Sep 22, 2017 • edited Loading

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

Choose a reason for hiding this comment

ro0NL commented Sep 26, 2017

Choose a reason for hiding this comment

ro0NL Oct 2, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabpot commented Oct 3, 2017

ro0NL commented Sep 10, 2017 •

edited

Loading

ro0NL Sep 11, 2017 •

edited

Loading

ro0NL Sep 11, 2017 •

edited

Loading

ro0NL commented Sep 12, 2017 •

edited

Loading

ro0NL commented Sep 12, 2017 •

edited

Loading

yceruto Sep 22, 2017 •

edited

Loading

ro0NL Oct 2, 2017 •

edited

Loading