Support for setting PHP settings through configuration

[nikolaposa]

Per-environment PHP configuration is very common use-case and so far it has been usually implemented by placing inline ini_set calls within some php_settings.php configuration itself. It would be much nicer if we can have clean configurations, for example:

return [
    'php_settings' => [
        'display_startup_errors'        => false,
        'display_errors'                => false,
        'date.timezone'                 => 'Europe/Belgrade'
    ]
];

In order to achieve that, PHP settings must be set at some point, and I think that the most suitable context is the ApplicationFactory.

[Maks3w]

this foreach should be wrapped in a try/finally where try is this loop and finally is the tear down loop

[Maks3w]

TBH I think environment configuration is something must be do in configuration scripts (vagrant, docker, ...)

In special since there are many settings can't be changed at runtime.

[nikolaposa]

@Maks3w What do you think about this approach, of having a helper method for configuring container based on the test specificity? Test cases now look more cleaner. Maybe these should be applied to all the other other ApplicationFactory tests.

[Maks3w]

While I saw too a helper or many helper methods are needed I disagree with this proposal because add too much logic.

The problem with add logic branches in a test case (if, for, ...) is difficult to trust is bugfree. What test method test getContainer works as expected?

[nikolaposa]

But this would allow us to have only one place for managing and maintaining container configuration and it also solves code duplication in all the test cases in this class. I agree though that it is somewhat unreliable due to its complexity, but after all, every test case which consumes will fail if it does not work as expected.

[geerteltink]

Sure it would look nice, but needed? In stead you can easily add a ./config/autoload/local.php file with a few ini_set calls in it.

Or maybe add some logic in the config loader that detects and sets the php_settings. This way the settings are set even before the application is started. In case of display_startup_errors and display_errors this is what you probably want anyway. I mean if you want to set display_errors to false and there is an error before it is actually set to false, the error is displayed.

[nikolaposa]

file with a few ini_set calls in it.

The very intent of this PR was to avoid just that, I explained it in the introductory comment.

This way the settings are set even before the application is started

I think that there is no point to track errors before this line: https://github.com/zendframework/zend-expressive-skeleton/blob/master/public/index.php#L18. Errors in pre-run stage can happen within the framework itself, unless of course someone put some massive, dynamic logic into configurations.

[Maks3w]

The alternative is use a different front controller (index_dev.php) with this kind of customizations.

For me the problem is this feature does not work with any setting, only settings which allow runtime changes.

[nikolaposa]

Ok, as for me, feel free to close this PR if you think that there will be no more comments or evaluations.

[nikolaposa]

... btw maybe at least we can consider fixing those inconsistencies regarding throwing Exception vs silent ignoring and eventually that helper method in ApplicationFactory test case which solves code duplication.

[Maks3w]

I've open the issue #196 for track the inconsistencies.

@nikolaposa Do you want to propose a PR for fix it?

[nikolaposa]

@Maks3w Why not. I'll send PR soon.

[Maks3w]

@nikolaposa One important thing @weierophinney point me exceptions thrown must implement Interop\Container\Exception\ContainerException

We could reuse https://github.com/nikolaposa/zend-expressive/blob/feature/php-settings/src/Container/Exception/InvalidServiceException.php or create an specific one.

[nikolaposa]

@Maks3w Duly noted.

[weierophinney]

I honestly don't think this should be done as part of the application factory.

INI settings are an environmental concern, and should be done typically in one of the following places, in descending order of preference:

• system-wide INI file
• virtual host
• .htaccess
• bootstrap script

The rationale? You want the environment completely setup before you execute any of your application code, to ensure consistency of execution. As others have noted on this thread, a number of INI settings CANNOT be done in runtime regardless, and those that can will affect all code following.

Considering the number of deployment options available to PHP developers today, such changes can be easily accommodated as part of deployment:

• ZPK (Zend Server packaging format) allows defining INI settings.
• Chef, Puppet, and Ansible all let you control this as part of your application container configuration.

In a worst-case scenario, I'd create a script for setting any application-specific INI settings you rely on that can be set at runtime, and including that from the top of your public/index.php script; however, with the plethora of other options, that really should be a worst-case scenario.

Regarding the specifics that led to this request (inability to track where bad configuration was leading to invalid application state): I believe these have now been addressed in #194 and #195, and #197 will address the final lingering issues.

[nikolaposa]

@weierophinney Thanks for this detailed explanation, now I'm convinced that this proposal has no practical value.