[SESSION] Configuration is not correct #314
Conversation
|
|
||
| ## Session handler, by default set to file based (instead of ~) in order to be able to use %ezplatform.session.save_path% | ||
| # env: SESSION_HANDLER_ID | ||
| ezplatform.session.handler_id: session.handler.native_file |
There was a problem hiding this comment.
afaik this will break: symfony/symfony-standard@070e53c
There was a problem hiding this comment.
then we have an issue because how would you override this config to "use the php.ini"?
In the link you mentioned they want to use the save_path setup in Symfony.
Here we want to let the php.ini handles the handler and the save_path.
It does not break to me, when both are null then php.ini will be used for both configuration.
There was a problem hiding this comment.
Could be a simple thing like this:
if (($value = getenv('SESSION_HANDLER_ID')) !== false) {
$container->setParameter('ezplatform.session.handler_id', $value ?: null);
}
If all you need is to set it to null via env
There was a problem hiding this comment.
@Plopix Or:
diff --git a/app/config/env/generic.php b/app/config/env/generic.php
index 72ccf2be..7e4f69a7 100644
--- a/app/config/env/generic.php
+++ b/app/config/env/generic.php
@@ -69,5 +69,5 @@ if ($value = getenv('LOG_TYPE')) {
}
if ($value = getenv('SESSION_HANDLER_ID')) {
- $container->setParameter('ezplatform.session.handler_id', $value);
+ $container->setParameter('ezplatform.session.handler_id', $value === '~' ? null : $value);
}
| httpcache_default_ttl: '%env(HTTPCACHE_DEFAULT_TTL)%' | ||
|
|
||
| # Session save path as used by symfony session handlers (eg. used for dsn with redis) | ||
| ezplatform.session.save_path: '%env(SESSION_SAVE_PATH)%' |
There was a problem hiding this comment.
why is this moved to generic.php?
There was a problem hiding this comment.
But that is in-consistent ;) Only params for handlers are done there now, as they can't be done at runtime.
kernel does not need to hard code usage of it, it's passed on to symfony here:https://github.com/ezsystems/ezplatform/blob/master/app/config/config.yml#L68-L69 |
|
About your last comment, as mentioned here: ezsystems/ezpublish-kernel#2410 The kernel does NOT use the @andrerom, Am I completely mistaking here? |
The intention of this code is to get the param from Symfony framework config, which in turn should have been set by the param. |
|
Oh right, there is an automated injection of a parameter "session.save_path" by symfony: https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php#L772 I closed the other PR, this one still has sense, right? |
Partly yes, but please get rid of the non bug related changes. |
|
|
||
| $container->setParameter('ezplatform.session.handler_id', 'ezplatform.core.session.handler.native_redis'); | ||
| $container->setParameter('ezplatform.session.save_path', sprintf('%s:%d', $endpoint['host'], $endpoint['port'])); | ||
| $container->setParameter('ezplatform.session.save_path', sprintf('tcp://%s:%d', $endpoint['host'], $endpoint['port'])); |
There was a problem hiding this comment.
It works without 'tcp://' too right?
Cause it won't work with memcached if we hardcode tcp://
There was a problem hiding this comment.
True, so maybe better to keep it as is then so we use same on SESSION_SAVE_PATH usage as on p.sh.
3 participants
This PR associated with: ezsystems/ezpublish-kernel#2410
Define that
php.iniis the default configuration.Then you can update that configuration through env. (current is
fileby default and that is not possible to override it (causenullvalue would NOT pass in theifingeneric.php)Also the kernel does not use
ezplatform.session.save_pathat all, then I am really confused.It fixes also the save path for redis
tcp://