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

Add redis pubsub pool and examples #71

Closed
wants to merge 2 commits into from
Closed

Add redis pubsub pool and examples #71

wants to merge 2 commits into from

Conversation

sync
Copy link

@sync sync commented May 23, 2020

The Redis crate recently added redis pubsub subscription and would like to use this feature with bb8.

https://github.com/mitsuhiko/redis-rs/blob/master/examples/async-pub-sub.rs

PS: I am still super new to rust so probably can be cleaned up

@@ -15,3 +15,4 @@ redis = "0.16"

[dev-dependencies]
tokio = { version = "0.2", features = ["macros"] }
futures-util = "0.3"
Copy link
Author

Choose a reason for hiding this comment

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

this is just need for the new redis examples, let me know if there is a better way of doing this ?


#[async_trait]
impl bb8::ManageConnection for RedisPubSubConnectionManager {
type Connection = PubSub;
Copy link
Author

Choose a reason for hiding this comment

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

need to do 2 things, change connection type to PubSub

self.client
.get_async_connection()
.await
.map(|conn| conn.into_pubsub())
Copy link
Author

@sync sync May 23, 2020

Choose a reason for hiding this comment

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

turn async connection into a pusbub connection into_pusbsub()

async fn is_valid(&self, conn: &mut Self::Connection) -> Result<(), Self::Error> {
let test_sub_name = "__test__-sub-from-bb8";
conn.subscribe(test_sub_name).await?;
conn.unsubscribe(test_sub_name).await
Copy link
Author

Choose a reason for hiding this comment

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

this feels hackish, but there is not much that can be done on a pubsub connection, basic CMD aren't available for example

@sync sync marked this pull request as ready for review May 23, 2020 06:23
@djc
Copy link
Owner

djc commented May 25, 2020

I'm not really comfortable with supporting a pubsub pool as part of bb8-redis. It seems like a relatively niche use case and it's not at all clear that (most usages of) pubsub connections are a good match for pooling because the length of the "loan" from the pool could be relatively high. I think dedicated_connection() is a better match, as @khuey suggested. If you want a pubsub pool, you can of course always maintain one inside your application or as part of a separate crate.

@djc djc closed this May 25, 2020
@sync
Copy link
Author

sync commented May 25, 2020

not a problem and thanks for the suggestion

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

Successfully merging this pull request may close these issues.

2 participants