-
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