shotover / shotover-proxy

L7 data-layer proxy
https://docs.shotover.io
Apache License 2.0
87 stars 18 forks source link

Port redis pubsub tests to fred #1537

Open rukai opened 8 months ago

rukai commented 8 months ago

In order to update the redis dependency used by our test suite we will need to migrate the pubsub tests to use a different redis client. This is because the new redis crate changed its pubsub support to disallow converting a connection between a pubsub connection and a regular connection. This is a very reasonable position for them to take since the redis pubsub protocol is very silly and this lets them sidestep a bunch of its silliness.

However its a bit annoying for us since we need to test the cornercases of this silliness.

From a quick look at fred it looks like they do support pubsub/nonpubsub on the same connection: https://docs.rs/fred/latest/fred/clients/struct.RedisClient.html#method.message_rx https://docs.rs/fred/latest/fred/clients/struct.RedisClient.html#method.subscribe

We should be able to maintain some of our existing pubsub tests with redis crate and then add extra tests with fred.

If fred doesnt work out we should consider writing a super minimal client on top of redis-protocol, shouldnt be too bad.

rukai commented 1 day ago

I've marked this as a good first issue, since converting the pubsub tests to run with fred seems like a reasonably scoped problem. If it turns out that fred doesnt work for this, then we will need to write a custom client to test pubsub which is no longer a good first issue.