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

ISubsriber.WithPrefix and IConnectionMultiplexer.WithPrefix implementations #896

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

pecanw
Copy link

@pecanw pecanw commented Jul 25, 2018

No description provided.

@mgravell
Copy link
Collaborator

For @NickCraver's benefit: this is related to an email discussion; I know this is something we've dismissed in the past, but I'm happy to look through the code and reconsider whether we should embrace it.

@mgravell mgravell self-assigned this Jul 25, 2018
@NickCraver
Copy link
Collaborator

I get the concept here (and agree with it), but seems like there has to be a much simpler implementation than constantly adding (or forgetting to add) to all wrappers. I'm curious on the scoping use cases - is this temporary, or for the life of the application? If the latter, it seems like a config option for suffice.

Curious to hear more, only issue I'd have here is maintenance/testing...it'd add a bunch.

@mgravell
Copy link
Collaborator

Valid question. The scenario discussed on email was multi-tenancy, so very similar to our own usgae re keys.

@pecanw
Copy link
Author

pecanw commented Jul 26, 2018

The main idea behind is having one connection multiplexer as a singleton in the application. We would like to add some profiling and monitoring on it. It is also quite convenient to have just one connection string to Redis (i.e. configuration) per application and changing it for each tenant.

Current solution allows having multiple connection multiplexers with different channel prefixes (set in configuration), but you have to wrap the database object to use the same prefix. In other words, it is a little bit awkward to have a different approach for prefixing channels and keys.

@NickCraver NickCraver added this to the 2.1 milestone Aug 28, 2018
@NickCraver NickCraver changed the base branch from master to main June 20, 2020 13:27
@NickCraver
Copy link
Collaborator

I've simplified this removing the ConnectionMultiplexer overhead, to where a user would cache a subscriber and a database if they wanted the pair, e.g. down to .WithChannelPrefix() on the public API (though maybe this should just be .WithPrefix(), dunno.

But the way it's prefixing is drastically different than RedisKey these days which has it in the struct. I think we should make the RedisChannel struct prefix aware and parallel here - @mgravell thoughts?

@DeepWorksStudios
Copy link

DeepWorksStudios commented Oct 19, 2023

When will this be completed?

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.

None yet

4 participants