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

Allow to specify node_connection_info for redis sentinel #369

Conversation

ederuiter
Copy link
Contributor

This allows you to select a different database/username/password for the connections to the underlying redis servers.

Closes #367

@ederuiter
Copy link
Contributor Author

Build failures seem unrelated to these changes; they all fail building a dependency

error: could not compile `value-bag` (lib) due to 3 previous errors

@bikeshedder
Copy link
Collaborator

Yes, those errors are completely unrelated.

It's quite a bummer that the dependencies keep breaking the MSRV checks of the deadpool crates. 😞

@bikeshedder
Copy link
Collaborator

btw. I'd just call that field node_connection_info to keep it in line with redis-rs:

Apart from that I don't see anything wrong with the PR. It's nitty picky, I know. I just don't think adding the sentinel_ prefix adds anything here except for extra verboseness when using the now public API.

…ection_info field to align with the redis crate
@ederuiter ederuiter force-pushed the feature/sentinel-node-connection-info branch from 1238274 to 3438ec9 Compare November 28, 2024 08:58
@ederuiter
Copy link
Contributor Author

Makes sense; I amended my commit with the changes

@ederuiter ederuiter changed the title Allow to specify sentinel_node_connection_info for redis sentinel Allow to specify node_connection_info for redis sentinel Nov 28, 2024
@bikeshedder bikeshedder merged commit eadd120 into deadpool-rs:master Nov 28, 2024
58 of 64 checks passed
@ederuiter
Copy link
Contributor Author

@bikeshedder would it be possible to release a new version of deadpool-redis?

@bikeshedder
Copy link
Collaborator

I'm currently hesitant releasing a version with #357 being merged which adds a feature that I'm going to rework as part of #226. I could either revert #357 and make a release that only adds this PR or release a version that also includes a change that is going to be reverted/reworked.

I'm thinking of going the middle ground. Reworking #357 so it just adds the timeout config which can be kept even when PubSub support lands in deadpool-redis. 😅

I just need a moment to make those changes. I'll release a new version of deadpool-redis asap.

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.

Unable to switch to a different redis database when using deadpool-redis with sentinel
2 participants