-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[HttpFoundation] Precalculate session expiry timestamp #33169
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
[HttpFoundation] Precalculate session expiry timestamp #33169
Conversation
0aacdcd
to
cc3dc65
Compare
3ac6cf7
to
1d7737c
Compare
The migration path doesn't work IMHO. |
1d7737c
to
e4f4a71
Compare
src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php
Outdated
Show resolved
Hide resolved
37a924c
to
1e5586d
Compare
src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php
Show resolved
Hide resolved
src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php
Show resolved
Hide resolved
1e5586d
to
9dd3b93
Compare
src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php
Show resolved
Hide resolved
e849cbb
to
0d57343
Compare
Co-authored-by: Benjamin Cremer <b.cremer@shopware.com> Co-authored-by: Rob Frawley 2nd <rmf@src.run>
0d57343
to
066000a
Compare
Thank you @azjezz. |
… (azjezz) This PR was merged into the 4.4 branch. Discussion ---------- [HttpFoundation] Precalculate session expiry timestamp | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | #13955, #21423, #14625 | License | MIT Continued work from the original PR #21423 / #21857 Commits ------- 066000a [HttpFoundation] Precalculate session expiry timestamp
Hello @nicolas-grekas, I think this is a BC break. On mysql, the lifetime column has been for the last few years a MEDIUMINT column. The new version with When upgrading from symfony 4.3 to 4.4 with an existing database, we have the error I saw the creation of the session table has been fixed 5 days ago in #34410 but that does not fix existing databases. Is it normal we need to create manually a sql migration for a breaking database schema change, which is not really mentioned in UPGRADE-4.4.md ? |
That's an interesting feedback @tseho but please create an issue: nobody is tracking comments on closed PRs. |
Thanks for your response and sorry about that, I just created a new issue : #34491 |
… session (IonBazan) This PR was squashed before being merged into the 6.0 branch. Discussion ---------- [HttpFoundation] [PDO] Don't fetch time when reading the session | Q | A | ------------- | --- | Branch? | 6.0 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | | License | MIT | Doc PR | Following up #33169 (comment) BTW is there a reason we are using `FETCH_NUM` instead of `FETCH_ASSOC` for more readability? Commits ------- fb9508f [HttpFoundation] [PDO] Don't fetch time when reading the session
Continued work from the original PR #21423 / #21857