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

[WIP] [Feature] Adding Redis as a Session and Cache handler #7206

Merged
merged 10 commits into from Nov 9, 2018

Conversation

Projects
None yet
2 participants
@deek87
Copy link
Contributor

deek87 commented Oct 16, 2018

This PR will do the follow things (Redis will only be available as PHPRedis extension at the moment)

  • Adds Redis as a Page Cache Option
  • Adds Redis as a Expensive/Request/Object Cache Option
  • Adds the ability to select a preferred cache driver (instead of forcing the overwrite of core_filesystem
  • Adds Redis as a SessionHandler (backported from symfony 4.1)
  • Allow Users to specify prefixes and the database
  • Allow users to select redis via the install page
  • Allow users to select redis via the dashboard
  • Add some tests for caching
  • Run PHP-CS Fixer

If there are other features of Redis you would like to see let me know, after this I will add Memcached as a page cache/object cache etc option

@aembler

This comment has been minimized.

Copy link
Member

aembler commented Nov 2, 2018

This looks fantastic – any thinking about when the last items might be done? Is this something we should accept as is without the public UI (choose redis during installation, etc)

@deek87

This comment has been minimized.

Copy link
Contributor

deek87 commented Nov 3, 2018

@aembler without the public stuff I can have it finished on monday i just need to update the sessionfactory to match the new style that was introduced by #7258

deek87 added some commits Nov 5, 2018

Merge branch 'develop' into feature/redis-cache
# Conflicts:
#	concrete/src/Session/SessionFactory.php
Updating SessionFactory
Refactoring Stash and Cache

@aembler aembler requested a review from KorvinSzanto Nov 8, 2018

@aembler

This comment has been minimized.

Copy link
Member

aembler commented Nov 8, 2018

This looks great to me. The only real issue I have is the methods for getting the file handler and the redis handler living within the SessionFactory itself – I'm wondering if they should be in separate classes. But I don't think that's a dealbreaker (I'll defer to @KorvinSzanto ). This is a great addition, to be able to have redis manage the full page caching and the expensive cache.

@deek87

This comment has been minimized.

Copy link
Contributor

deek87 commented Nov 9, 2018

@aembler I think it might be best to use a similar way to page cache which allows us to load things like MemcachedSessionProvider.php from either the core directory or the application and stops the the session factory becoming bloated. That's a different PR though

@aembler aembler merged commit 2b8e153 into concrete5:develop Nov 9, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment