Skip to content

[DI] Fix unresolved env parameters values #20695

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

[DI] Fix unresolved env parameters values #20695

wants to merge 1 commit into from

Conversation

chalasr
Copy link
Member

@chalasr chalasr commented Nov 30, 2016

Q A
Branch? master
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? not yet
Fixed tickets #20684
License MIT
Doc PR n/a

This is a poc to make env parameters values resolved as any parameter when using EnvPlaceholderParametereBag::resolveValue(), moving the logic of fetching env values from the container to the parameter bag. This needs work, concerns should be separated to don't mix placeholders resolution and values resolution (i.e. EnvParameterBag). It makes the debug:config command dumps configuration with the actual values of env parameters (default value is used if not defined as env var) and is probably useful for the large number of use cases where parameters need to be resolved.

Is it something viable? I'm looking for inputs.
ping @nicolas-grekas

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Nov 30, 2016 •
edited
Loading

It looks like it's breaking all the mechanism, which is based on injecting uniquelly identifiable strings into extensions so that we can resolve them later...
Also, the linked issue already have a PR for the buggy part, and an implementation hint for the "new feat" part.
I don't get this PR :)

@chalasr
Copy link
Member Author

chalasr commented Nov 30, 2016

The goal is to have a similar behavior than normal parameters for env ones, which is actually not possible (resolving all env parameters in a given value) but if it breaks everything, fair enough.

an implementation hint for the "new feat" part.

I would like to have the actual parameter value in debug:config instead of the default one which is not really useful when debugging :/ If it is what you suggested, I didn't get it

@nicolas-grekas
Copy link
Member

See #20618, second commit especially.

@chalasr
Copy link
Member Author

chalasr commented Nov 30, 2016

Indeed, I missed this one!

@chalasr chalasr closed this Nov 30, 2016
@chalasr chalasr deleted the resolve_env_values branch November 30, 2016 10:37
@chalasr chalasr mentioned this pull request Jan 27, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Bug Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@chalasr @nicolas-grekas @carsonbot