Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[HttpFoundation] Fix PdoSessionHandler to work properly with streams#12146
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
every method can throw this. so this is not really helpful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I added it because PhpStorm shows a warning otherwise: "PHPDoc comment doesn't contain all necessary@throws tag(s)". Should I remove it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yes because this doesnt only apply to this method.
Tobion commentedOct 6, 2014
Did you test this class with a real sqlite connection? If so could you please also test the blocking withhttps://github.com/Tobion/symfony-standard/tree/session-locking-test ? |
rybakit commentedOct 6, 2014
I didn't test it with a real sqlite connection as I don't use custom session handlers in my project. I've installed your |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Can you move everything on one line? Same below, thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Done.
Tobion commentedOct 6, 2014
Did you updatehttps://github.com/Tobion/symfony-standard/blob/session-locking-test/app/config/config.yml#L46 to use postgres? You can test blocking by loading two pages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
errorMode and driverName can be private
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Done.
rybakit commentedOct 6, 2014
@Tobion I've tested |
Tobion commentedOct 6, 2014
Cool thanks. And did pdo_pgsql indeed return a stream for the binary field? |
rybakit commentedOct 6, 2014
Yes, and it's mentioned in themanual:
|
Tobion commentedOct 6, 2014
👍 Thanks for fixing this and doing functional tests. |
fabpot commentedOct 7, 2014
Thank you@rybakit. |
Ref:#10931 (comment)