Redis sans EventMachine #23381
Redis sans EventMachine #23381
Conversation
This new adapter does get a little more intimate with the redis-rb gem's implementation than I would like, but it's the least bad of the approaches I've come up with.
|
|
||
| class RedisAdapterTest::Hiredis < RedisAdapterTest | ||
| def cable_config | ||
| super.merge(driver: 'hiredis') |
There was a problem hiding this comment.
Does setting driver configure faye-websocket somehow? Because I don't understand it 😁
There was a problem hiding this comment.
It's an option for the Redis library, which I want to ensure we work with.
|
Fantastic 🤘 |
| @@ -65,6 +65,7 @@ group :cable do | |||
| gem 'puma', require: false | |||
|
|
|||
| gem 'em-hiredis', require: false | |||
| gem 'hiredis', require: false | |||
There was a problem hiding this comment.
What is the reason to have hiredis here?
There was a problem hiding this comment.
It's used here to ensure we work with that driver as well.
There was a problem hiding this comment.
But isn't hiredis a dependency for em-hiredis? Do we need both?
There was a problem hiding this comment.
I added it to be clear that we're using hiredis directly somewhere, separate from the place that uses em-hiredis. Of course, test failures would tell us that too, so it doesn't really matter. ¯_(ツ)_/¯
There was a problem hiding this comment.
I'm fine with both, so lets keep this way.
|
|
This new adapter does get a little more intimate with the redis-rb gem's implementation than I would like, but it's the least bad of the approaches I've come up with.
Apologies to anyone who was (privately or publicly) disappointed this wasn't included in #23305; it just had enough complexity to deserve separate attention.