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

Refresh Parameters #10674

Closed
pierredup opened this issue Apr 9, 2014 · 8 comments
Closed

Refresh Parameters #10674

pierredup opened this issue Apr 9, 2014 · 8 comments

Comments

@pierredup
Copy link
Contributor

Many bundles are dependant on parameters being defined and passed through from the config.
When updating any parameters, the cache needs to be cleared first, as the parameters are cached in the generated container class.
This makes it difficult for an application to offer ability to change config parameters.

For example, the swiftmailer settings is defined in the parameters.yml, so if you want to change your smtp password for instance, you need to manually edit the parameters.yml file to make the change, then refresh the cache.

For most applications, it would be useful to have an interface to be able to edit the parameters, and have a way to refresh the parameters from the cache without having to run a command form the console or deleting the cache dir contents.

A good example is Sylius, which require quite a lot of parameters to be set in the parameters.yml file.

I would create a PR, but first need to find out what is the best way to handle this.

My suggestions is to cache all the parameters in a parameters.php file, which can then just be included in the generated getDefaultParameters() function, and have a function either on the container or the kernel called refreshParameters(), which simply regenerates this file.

This way a bundle can have an interface to update certain values in the parameters.yml file, and have that values updated in the container without clearing all the cache.

@jakzal
Copy link
Contributor

jakzal commented Apr 9, 2014

👎 as it would unnecessarily complilcate the configuration and potentially decrease the performance. It's not really needed in most of projects and there are third party bundles making this possible: https://github.com/opensky/OpenSkyRuntimeConfigBundle

@pierredup
Copy link
Contributor Author

The only complicated part would be to generate the parameters in a different file, which isn't actually so complicated. There would be no complication on the user's side, and it won't have any effect on any existing functionality. The only difference would be one adding extra public method that a user can decide to use if they want to.
And performance will be unaffected, as the only change would be a require_once call inside the generated container file (adding maybe a few nano seconds to the request).

The issues with bundles like the OpenSkyRuntimeConfigBundle, is that the parameters needs to be stores somewhere else (E.G in a database). For new projects, it's fine to use that, but for existing projects like Sylius, it will require a lot of work and rewrites to incorporate that functionality and move away from parameters the yml file.
Also you won't be able to inject the parameters required by Symfony's core services, (E.G swiftmailer), as they are dependant on the parameters being available in the container and not injected from a database.

@mvrhov
Copy link

mvrhov commented Apr 10, 2014

@jakzal. This doesn't really work as you can't really configure 3rd party bundles that way.
I was very excited when the expression engine landed in the 2.4 as I thought that I could also configure the bundles , but it also doesn't work as the configs got through config builder which then barfs.
Also the OpenSkyRuntimeConfigBundle is unnecessary since the expression engine landed in 2.4

edit: Well the @pierredup basically said what I said. This happens if you open the issue and then forget about it.

@mvrhov
Copy link

mvrhov commented Apr 10, 2014

@pierredup: It's not that simple. The getParameters method is there for your convenience only. The DIC doesn't call the function and the parameters are actually in-lined into the functions that need them.

@stof
Copy link
Member

stof commented Apr 10, 2014

@mvrhov is right, and this is where the perf impact will be. currently, parameters are all resolved at compile time, not at runtime

@stof
Copy link
Member

stof commented Apr 10, 2014

Thus, if you want to allow changing parameters at runtime including for parameters passed to the semantic configuration of bundles, this means that you have to rebuild the whole container (as you cannot know what the DI extension is doing with the value when building the container). This means a huge impact on perf

@pierredup
Copy link
Contributor Author

The generated container includes a function called getDefaultParameters which contains an array of all the parameters, and in the constructor, it is used by calling $this->parameters = $this->getDefaultParameters();. So for the inline parameters, we can change it to use $this->parameters[$name] instead (but not really sure of the implications of this).

Even if the container needs to be rebuilt, the performance impact will only be once-off for that request if the bundle updates any parameter. So it won't impact normal performance, and each developer can choose when and how and if they want to update the parameters. A note can also be added to the documentation that warns developers of the performance impact when updating the parameters and rebuilding the container.

@stof
Copy link
Member

stof commented Apr 10, 2014

@pierredup if a parameter is used in the semantic config of a bundle, it can impact the definition of services (loading more services, changing the class of a service, or anything else). This requires recreating the whole container, which means running your app in development mode (at least for the container building part)

@fabpot fabpot closed this as completed Oct 5, 2015
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

5 participants