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.

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

[SESSION] Configuration is not correct #314

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[SESSION] Configuration is not correct #314

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions app/config/default_parameters.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,6 @@ parameters:
# using EzSystemsPlatformHttpCacheBundle (default as of v1.12) which by design expires affected cache on changes
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)%'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this moved to generic.php?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to be consistent.

Copy link
Contributor

@andrerom andrerom Aug 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But that is in-consistent ;) Only params for handlers are done there now, as they can't be done at runtime.


# Recommendation Bundle params
ez_recommendation.default.yoochoose.customer_id: '%env(RECOMMENDATIONS_CUSTOMER_ID)%'
ez_recommendation.default.yoochoose.license_key: '%env(RECOMMENDATIONS_LICENSE_KEY)%'
Expand Down Expand Up @@ -95,9 +92,12 @@ parameters:
# env: HTTPCACHE_PURGE_TYPE
purge_type: local

## 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

afaik this will break: symfony/symfony-standard@070e53c

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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);
 }

## Session handler, by default relying on the php.ini
# env: SESSION_HANDLER_ID (to override it)
ezplatform.session.handler_id: ~
# Used when ezplatform.session.handler_id is not null
# env: SESSION_SAVE_PATH (to override it)
ezplatform.session.save_path: ~

# Admin siteaccess group name
admin_group_name: admin_group
4 changes: 4 additions & 0 deletions app/config/env/generic.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,7 @@
if ($value = getenv('SESSION_HANDLER_ID')) {
$container->setParameter('ezplatform.session.handler_id', $value);
}

if ($value = getenv('SESSION_SAVE_PATH')) {
$container->setParameter('ezplatform.session.save_path', $value);
}
4 changes: 2 additions & 2 deletions app/config/env/platformsh.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@
}

$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']));
Copy link
Member

@vidarl vidarl Aug 20, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works without 'tcp://' too right?
Cause it won't work with memcached if we hardcode tcp://

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, so maybe better to keep it as is then so we use same on SESSION_SAVE_PATH usage as on p.sh.

}
} elseif (isset($relationships['rediscache'])) {
foreach ($relationships['rediscache'] as $endpoint) {
Expand All @@ -114,6 +114,6 @@
}

$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']));
}
}