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

3.1 Session => ['defaults' => 'cake'] should try to auto-create tmp/sessions if not existing #7987

Closed
ionas opened this Issue Jan 7, 2016 · 7 comments

Comments

Projects
None yet
5 participants
@ionas
Contributor

ionas commented Jan 7, 2016

When switching from A:

    'Session' => [
        'defaults' => 'php',
    ],

to B:

    'Session' => [
        'defaults' => 'cake',
    ],

... because of the existence of an application cluster, the session path in tmp was not found and resulted in an error:

Warning (2): session_start(): open(/var/www/app/tmp/sessions/sess_4tkdpateu38p9l9n7iolj4qta1, O_RDWR) failed: No such file or directory (2) [CORE/src/Network/Session.php, line 324]

Should switching from A to B try to auto-create tmp/sessions path if not existing?

@ionas ionas changed the title from 3.1 Session => ['defaults' => 'cake'] should try to auto-create tmp/sessions if not existing (maybe in catch?) to 3.1 Session => ['defaults' => 'cake'] should try to auto-create tmp/sessions if not existing Jan 7, 2016

@markstory markstory added this to the 3.1.7 milestone Jan 8, 2016

@markstory

This comment has been minimized.

Show comment
Hide comment
@markstory

markstory Jan 8, 2016

Member

I am not so sure we want to try and create directories that don't exist. Unless we did some special casing, we'd be trying to ensure that session.save_path existed if it wasn't defined and we could run into issues around filesystem permissions or open_basedir limitations.

Member

markstory commented Jan 8, 2016

I am not so sure we want to try and create directories that don't exist. Unless we did some special casing, we'd be trying to ensure that session.save_path existed if it wasn't defined and we could run into issues around filesystem permissions or open_basedir limitations.

@htstudios

This comment has been minimized.

Show comment
Hide comment
@htstudios

htstudios Jan 8, 2016

Contributor

Within ROOT/tmp/ it should be okay?
Don't we do the same for cache, models, persistent etc?

Contributor

htstudios commented Jan 8, 2016

Within ROOT/tmp/ it should be okay?
Don't we do the same for cache, models, persistent etc?

@ADmad

This comment has been minimized.

Show comment
Hide comment
@ADmad

ADmad Jan 8, 2016

Member

tmp/sessions is already created at install time along with other tmp directories. We should definitely not try to create directories at runtime.

Member

ADmad commented Jan 8, 2016

tmp/sessions is already created at install time along with other tmp directories. We should definitely not try to create directories at runtime.

@ADmad ADmad closed this Jan 8, 2016

@ionas

This comment has been minimized.

Show comment
Hide comment
@ionas

ionas Jan 8, 2016

Contributor

The gap that surprises is, following directories get setup on install time:
ROOT/logs
ROOT/tmp
ROOT/tmp/cache
ROOT/tmp/cache/models
ROOT/tmp/cache/persistent
ROOT/tmp/cache/views
ROOT/tmp/sessions

"We should definitely not try to create directories at runtime" (@ADmad)

Following directories are created at runtime.
Thus they will be re-created (tested just now) if they got deleted:
ROOT/logs
ROOT/tmp
ROOT/tmp/cache
ROOT/tmp/cache/models
ROOT/tmp/cache/persistent
ROOT/tmp/cache/views
ROOT/tmp/sessions

Did not test for ROOT/tmp/tests, e.g. if it gets recreated on demand.

p.s.: I'd have preferred to discuss this in short on IRC.

Contributor

ionas commented Jan 8, 2016

The gap that surprises is, following directories get setup on install time:
ROOT/logs
ROOT/tmp
ROOT/tmp/cache
ROOT/tmp/cache/models
ROOT/tmp/cache/persistent
ROOT/tmp/cache/views
ROOT/tmp/sessions

"We should definitely not try to create directories at runtime" (@ADmad)

Following directories are created at runtime.
Thus they will be re-created (tested just now) if they got deleted:
ROOT/logs
ROOT/tmp
ROOT/tmp/cache
ROOT/tmp/cache/models
ROOT/tmp/cache/persistent
ROOT/tmp/cache/views
ROOT/tmp/sessions

Did not test for ROOT/tmp/tests, e.g. if it gets recreated on demand.

p.s.: I'd have preferred to discuss this in short on IRC.

@dereuromark

This comment has been minimized.

Show comment
Hide comment
@dereuromark

dereuromark Jan 8, 2016

Member

@ionas Folders are only created in debug mode, not production.
Also, tmp, logs, cache and tmp/sessions for CakePHP itself is fine , but another outside is not OK and should be done automatically.

See https://github.com/cakephp/cakephp/blob/master/src/Log/Engine/FileLog.php#L90-L95
The same for debug mode only should be applied to all the tmp stuff inside the APP.

Member

dereuromark commented Jan 8, 2016

@ionas Folders are only created in debug mode, not production.
Also, tmp, logs, cache and tmp/sessions for CakePHP itself is fine , but another outside is not OK and should be done automatically.

See https://github.com/cakephp/cakephp/blob/master/src/Log/Engine/FileLog.php#L90-L95
The same for debug mode only should be applied to all the tmp stuff inside the APP.

@ionas

This comment has been minimized.

Show comment
Hide comment
@ionas

ionas Jan 8, 2016

Contributor

In case there is a will for consistency let me know and I will try to fix it ASAP.

Contributor

ionas commented Jan 8, 2016

In case there is a will for consistency let me know and I will try to fix it ASAP.

@dereuromark

This comment has been minimized.

Show comment
Hide comment
@dereuromark

dereuromark Jan 8, 2016

Member

Please make a PR. I think that will make the discussion easier as it will be more code hands-on and less theoretical.

Member

dereuromark commented Jan 8, 2016

Please make a PR. I think that will make the discussion easier as it will be more code hands-on and less theoretical.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment