Skip to content

Setting client lib name properly by wrapping multiplxer creation #420

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

atakavci
Copy link
Collaborator

@atakavci atakavci commented May 12, 2025

Closes/Fixes #406

Current implementation for setting client lib info on connections is only working for single initial connection in the application context/runtime, which leaves the cases and setups given below out of scope;

  • client reconnects
  • pubsub subscription connections in RESP2 mode
  • cluster setup

There are couple of ways to improve the situation with setting library info on connections but all have its downsides. Here some of them with their own drawbacks;

  • using connections events
    • this doesnt provide a way to set subscription connections
    • need to manage event registration process for each and every connections explicitly, which is kind of unnecessary burden both in terms of performance and maintenance
  • renaming/overriding internals with unnatural/unreliable ways
    • this is risky to play with internals of other library
    • this means introducing a sophisticated code/solution for a simple problem, which would make it
    • this is a potential maintenance issue due to relying on unexposed API/parts of StackExchange.Redis assembly.
  • providing a high-level client initializer wrapping both database and multiplexer instances
    • this changes the initial point of contact of developer with the library API and change how the developer experiences
    • it brings maintenance work when the underlying API changes with IDatabase interface, this was handled in a zero-effort manner with help of extension methods previously

I choose to follow the third approach since it is clear, reliable and simplistic implementation compared to other two.
In this PR there are three major types introduced, two of them is exposed in public API;
IRedisDatabase : An interface to allow access to modules commands in addition to those in IDatabase
RedisClient : Provides creation+configuration of IRedisDatabase instances
DatabaseWrapper : Underlying wrapper class to compose IDatabase and modules commands into same type.

Existing implementation with extension methods will stay (at least until a major release) and we will keep these new types as experimental, until we make sure this approach brings more benefits, allow required modifications with underlying types and complies with up-coming changes with StackExchange.Redis.

@atakavci atakavci requested review from uglide and slorello89 May 12, 2025 06:53
@atakavci atakavci self-assigned this May 12, 2025
@atakavci atakavci added enhancement New feature or request experimental labels May 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request experimental
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setting client info does not work properly for re-connects and multiple connections
1 participant