Skip to content
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

[Fixes #213] Add support for the redis gem #280

Merged
merged 1 commit into from
Jun 29, 2018

Conversation

bfad
Copy link
Contributor

@bfad bfad commented Feb 6, 2018

While a cache-store proxy exists for the redis-store gem, no such proxy existed for using the redis gem itself. This fills that gap by adding such a proxy.

Resolves #190

While a cache-store proxy exists for the redis-store gem, no such proxy
existed for using the redis gem itself. This fills that gap by adding
such a proxy.

Resolves rack#190
@grzuy grzuy changed the title Add support for the redis gem (Resolves 190) [Fixes #190] Add support for the redis gem Mar 22, 2018
Copy link
Collaborator

@grzuy grzuy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @bfad,

Thank you for taking the time to work on this!

Left one comment/question.

Don't worry about the merge conflicts, I can take care of those before merging.


ACTIVE_SUPPORT_WRAPPER_CLASSES = Set.new(['ActiveSupport::Cache::MemCacheStore', 'ActiveSupport::Cache::RedisStore']).freeze
ACTIVE_SUPPORT_CLIENTS = Set.new(['Redis::Store', 'Dalli::Client', 'MemCache']).freeze
ACTIVE_SUPPORT_CLIENTS = Set.new(['Redis::Store', 'Redis', 'Dalli::Client', 'MemCache']).freeze
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure we need Redis here for the proxy to work.
Do you think we do?

@grzuy grzuy changed the title [Fixes #190] Add support for the redis gem [Fixes #213] Add support for the redis gem Jun 29, 2018
@grzuy grzuy merged commit 0f6ef47 into rack:master Jun 29, 2018
@grzuy
Copy link
Collaborator

grzuy commented Jun 29, 2018

Made the change myself and merged.

Thanks again! 🎉

@bfad
Copy link
Contributor Author

bfad commented Jun 29, 2018

Thanks for getting this in 🍾

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants