-
Notifications
You must be signed in to change notification settings - Fork 25
Update colinmollenhour/php-redis-session-abstract #39
Update colinmollenhour/php-redis-session-abstract #39
Conversation
fb7d357 to
68f9acc
Compare
| /** | ||
| * Try to break lock for at most this many seconds | ||
| */ | ||
| const DEFAULT_FAIL_AFTER = 15; |
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.
Why is this one not configurable? Comparing to PARAM_BREAK_AFTER?
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.
It's also necessary to update setup:config:set with new params (in \Magento\Setup\Model\ConfigOptionsList\Session).
Could you please also update http://devdocs.magento.com/guides/v2.2/config-guide/redis/redis-session.html ?
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 copied DEFAULT_FAIL_AFTER from 2.2-develop https://github.com/magento/magento2/blob/0839bd421bd5812e9aa7c943b89ecae9d5008832/lib/internal/Magento/Framework/Session/SaveHandler/Redis/Config.php#L121 Should I change it to be configurable here?
Thanks for pointing out the changes required to setup:config:set. I'll get that taken care of.
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.
Got it... ok, let's leave it as is.
68f9acc to
1babe34
Compare
* Update implementation of `Cm\RedisSession\Handler\ConfigInterface` * Add Redis Sentinel config options to `setup:config:set` command
1babe34 to
6b1ae42
Compare
Description
Update colinmollenhour/php-redis-session-abstract to version supporting PHP 7.2.
Fixed Issues (if relevant)
Manual testing scenarios
Contribution checklist