Skip to content

Improve %env()% parameters in debug:config output #20684

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

Closed
javiereguiluz opened this issue Nov 29, 2016 · 9 comments
Closed

Improve %env()% parameters in debug:config output #20684

javiereguiluz opened this issue Nov 29, 2016 · 9 comments

Comments

@javiereguiluz
Copy link
Member

javiereguiluz commented Nov 29, 2016

Problem

When using runtime env variables for config:

# app/config/config.yml
doctrine:
    dbal:
        url: "%env(DATABASE_URL)%"

# app/config/parameters.yml
parameters:
    env(DATABASE_URL): 'sqlite:///%kernel.root_dir%/data/blog.sqlite'

The output of debug:config looks like this:

$ ./bin/console debug:config doctrine

doctrine:
    dbal:
        default_connection: default
        connections:
            default:
                url: env_DATABASE_URL_b188317b1d181eca5f0be35aefdae9c4
                ...

Although the output is technically precise, it looks weird and even like an error.

Solution

Could we keep the original value of those parameters? (Maybe displaying their default values too)

$ ./bin/console debug:config doctrine

doctrine:
    dbal:
        default_connection: default
        connections:
            default:
                url: "%env(DATABASE_URL)%" # default: 'sqlite:///%kernel.root_dir%/data/blog.sqlite'
                ...
@nicolas-grekas
Copy link
Member

See #20688 for the unresolved part.
If someone wants to give it a try, displaying could be done using a combination of $container->resolveEnvPlaceholders()' $usedEnvs arg + $parameterBag->all() to get the default values.

@nicolas-grekas
Copy link
Member

I don't know if the Yaml dumper supports dumping comments as in the suggested excerpt.
I think it'd be much easier to dump the parameter section with default values for all env params that have one.
Btw, we don't display the value of other params, so why doing so for env params?

@chalasr
Copy link
Member

chalasr commented Nov 29, 2016

@nicolas-grekas debug:config actually dumps the config with resolved values for non-env parameters, not their placeholders. But maybe I misunderstand

@chalasr
Copy link
Member

chalasr commented Dec 1, 2016

@nicolas-grekas In fact, we are both right: they are only half resolved
See #20714

@nicolas-grekas
Copy link
Member

So, in #20704, I fix the buggy part, yet env vars are not replaced by their actual values for now.
To me, resolving the values, being for remaining unresolved regular params or env vars should be discussed and changed in 3.3. The current behavior is also useful so I don't know what's the best as far as DX is concerned.

@javiereguiluz
Copy link
Member Author

I'm closing this as "fixed" because I've tested this with 3.3 and 3.4 and it works as expected. Replacing the env vars by their actual values should be considered as a different feature. Thanks!

@curry684
Copy link
Contributor

This is actually still broken in Symfony 4.1.1 as it crashes validation.

For example if using this in a bundle configuration:

                ->scalarNode('my_api_url')
                    ->isRequired()
                    ->validate()
                        ->ifTrue(function ($val) { return !preg_match('#^https?://[^\s]+$#', $val); })
                        ->thenInvalid('"%s" must be a valid URL with http/https scheme')
                    ->end()
                ->end()

And then configuring this as:

  my_api_url: "%env(MY_API_URL)%"

The debug:config will crash:

  Invalid configuration for path "mybundle.my_api_url": ""%env(MY_API_URL)  
  %"" must be a valid URL with http/http scheme           

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Jun 30, 2018

@curry684 in this case that's expected: how dear would it be possible to validate at compile time a value that is known at runtime? Env vars are not compatible with every situations. At some point, this has to be supported case by case, option by option, by refactoring the code to use runtime factories that embed some validation. But only when it makes sense. Turning all options to dynamic factories would basically mean turning off compilation. Not wanted either.

@curry684
Copy link
Contributor

curry684 commented Jul 1, 2018

@nicolas-grekas I'm not saying it can be fixed by showing an unknown mutable value, I'm just saying it's not DX-friendly that it crashes completely. We could surely fix debug:config to instead print %env(MY_API_URL)% in a different color instead.

Also, in my specific case it wouldn't have had to crash as MY_API_URL was both present in %project_dir%/.env and available in the Docker container I was executing the command in.

There's certainly room for improvement there over a blunt validation exception, as it more or less obsoletes debug:config in many applications for many bundles 😉

But no, if it were an easy fix I would've made the PR already, or opened a separate issue, over remarking here that the discussed behavior is still broken in a lot of cases.

curry684 added a commit to curry684/symfony that referenced this issue Jul 2, 2018
As `debug:config` only resolved back to original placeholders these could then fail validation if present.

Refs symfony#20684 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants