-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Conversation
@Tobion remind me, was this something that changed in Symfony 3? I always have a hard time wrapping me head around this, because I know (at least previously), that setting this setting to |
Not defining |
What about merging this in 2.3 then ? EDIT: I've just seen |
@Tobion I agree with your point on DX. So, we used to default to But I think I like it: though what about keeping the comment that setting this to And yes, |
@ogizanagi the bug is ony in 3.0 Ok to sum up: In #606 the session handler was changed to use php.ini values. If you use the php.ini session handler, then the
So either we keep using php.ini settings by default (which means we do not know where sessions are saved and thus the var/session directory makes no sense). Or we want to use |
|
In fact, the final solution added by Fabien in 3.0, was suggested by drak (ghost, unfortunately, on the PR) on the linked #606 PR: #606 (comment)... but that was never the right solution (for the exact reasons this issue was opened). I think we should keep the native, php.ini handler, but with more comments than we have now: session:
# handler_id set to null will use the default session handler from php.ini
handler_id: ~
# or, you can specify some specific handler and options
# http://symfony.com/doc/current/reference/configuration/framework.html#handler-id
# handler_id: session.handler.native_file
# save_path: "%kernel.root_dir%/../var/sessions/%kernel.environment%" |
But then it does not make sense to preconfigure the directory https://github.com/symfony/symfony-standard/tree/master/var/sessions if it is not used. |
@Tobion you're right, so I would vote to remove the If we do use my proposal and you think the code is a little to wordy (i.e. too many comments), then yea: we could just link to the docs for how to configure session handlers in general. We should link to the reference docs - the link you posted (unfortunately) only shows you how to do file storage: it doesn't help anyone looking to configure something else, like Redis. So we're back to what you mentioned @Tobion: do we default to using the php.ini default (as was done in 2.4-3.0 in the SE) or do we default to |
As I understand it, @fabpot proposal was only about avoiding to have the But then, as @weaverryan expressed, it makes no sense to have the I would opt for @weaverryan suggestion. Or even remove back the |
Imo having sessions stored in |
The more I think about this, the more I agree with @xabbuh: put things in session:
# http://symfony.com/doc/current/reference/configuration/framework.html#handler-id
handler_id: session.handler.native_file
save_path: "%kernel.root_dir%/../var/sessions/%kernel.environment%" 👍 as-is, but even more with the docs link comment |
I've got the same issue (symfony/symfony#17157). Please merge this! |
Thank you @Tobion. |
This PR was merged into the 3.0 branch. Discussion ---------- fix ignored session save_path fixes symfony/symfony#16898 do not use php.ini session handler but native file to actually use save_path config When handler_id is null it uses the Native one, but that does not set the save_path. So we need to use https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/NativeFileSessionHandler.php instead (which is the default configuration). Commits ------- 070e53c do not use php.ini session handler but native file to actually use save_path config
@weaverryan added the doc mention in 44a1fd8 |
fixes symfony/symfony#16898
do not use php.ini session handler but native file to actually use save_path config
When handler_id is null it uses the Native one, but that does not set the save_path. So we need to use https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/NativeFileSessionHandler.php instead (which is the default configuration).