-
-
Notifications
You must be signed in to change notification settings - Fork 436
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Add redis as a valid option for global/session_save #1513
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
Component: Cm/RedisSession
Relates to Cm_RedisSession
Component: Core
Relates to Mage_Core
labels
Mar 29, 2021
Thanks for the PR! It would probably require some care in the future not to clobber your changes when updating the module but updates are also quite rare.. :) I'm in favor for 20.x to make it enabled out of the box and hence a little easier to configure. |
colinmollenhour
approved these changes
Apr 1, 2021
fballiano
approved these changes
Apr 5, 2021
kkrieger85
approved these changes
May 12, 2021
3 tasks
This PR causes issues with <config>
<modules>
<Cm_RedisSession>
<version>0.2</version>
</Cm_RedisSession>
</modules>
<global>
<models>
<core_mysql4>
<rewrite>
<session>Cm_RedisSession_Model_Session</session>
</rewrite>
</core_mysql4>
<!-- This causes issues with Amasty_Fpc
<cm_redissession>
<class>Cm_RedisSession_Model</class>
</cm_redissession>
-->
</models>
</global>
</config> |
Here's a patch. diff --git a/app/code/local/Amasty/Fpc/Model/Fpc/Front.php b/app/code/local/Amasty/Fpc/Model/Fpc/Front.php
index bb83b66..cbd7e78 100644
--- a/app/code/local/Amasty/Fpc/Model/Fpc/Front.php
+++ b/app/code/local/Amasty/Fpc/Model/Fpc/Front.php
@@ -464,6 +462,10 @@ class Amasty_Fpc_Model_Fpc_Front extends Varien_Object
}
switch ($moduleName) {
+ case 'redis':
+ $sessionResource = new Amasty_Fpc_Model_Resource_Redis_Session();
+ $sessionResource->setSaveHandler();
+ break;
case 'db':
if ($this->isModuleEnabled('Cm_RedisSession'))
$sessionResource = new Amasty_Fpc_Model_Resource_Redis_Session(); Amasty_Fpc has a few issues with OpenMage. It needs a session namespace patch too. |
@rjocoleman Thanks for the patch! |
justinbeaty
added a commit
to justinbeaty/Cm_RedisSession
that referenced
this pull request
Aug 3, 2022
colinmollenhour
pushed a commit
to colinmollenhour/Cm_RedisSession
that referenced
this pull request
Aug 3, 2022
* Remove submodules * Do not rewrite (ref: OpenMage/magento-lts#1513) Co-authored-by: Justin Beaty <justin.beaty@gmail.com>
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Labels
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a backwards incompatible change, so I don't think it's a good idea to merge it into 1.9.4.x, however 20.0 perhaps..?
So, this PR:
redis
as an entry in the Core session selection switch statement.Cm_RedisSession
(to restatedb
whenCm_RedisSession
is enabled)Cm_RedisSession
The impact of this is in
local.xml
global/session_save
will accept bothdb
andredis
as valid values.The reason this is backwards incompatible is
Cm_RedisSession
used to rewrite and replace thedb
handler (ifCm_RedisSession
was set to enabled/active in its module config.Thus if merged
Cm_RedisSession
users will revert to DB for session storage.Description (*)
This was mentioned here (among other places): #379 (comment)
Related Pull Requests
Fixed Issues (if relevant)
Manual testing scenarios (*)
<global><session_save>
toredis
local.xml
redis_session
redis-cli FLUSHALL
(etc) to drop clear the redis database (this will clear the whole redis db so don't run it on something you care about)Questions or comments
Contribution checklist (*)