Repository navigation
Set LibraryVersion in ConfigurationOptions #2533
Description
I saw that now we can set the LibraryName in the ConfigurationOptions, what about LibraryVersion?
can we add LibraryVersion as well like here?
I saw that now we can set the LibraryName in the ConfigurationOptions, what about LibraryVersion?
can we add LibraryVersion as well like here?
Activity
[-]LibraryVersion in ConfigurationOptions[/-][+]Set LibraryVersion in ConfigurationOptions[/+]NickCraver commentedon Aug 29, 2023
What is the use case for changing it? The library name being modifiable to add more per-wrapping-application info was the intent with making it changeable, but the version is what we compile here and has a lot of value on the backend.
shacharPash commentedon Aug 30, 2023
@NickCraver In NRedisStack I want to send the lib-name and lib-ver automatically (unless otherwise specified) when the client creates a connection, and I can easily do this with ConfigurationOptions if I can also set the
LibraryVersion.NickCraver commentedon Sep 1, 2023
@shacharPash I think that makes sense, it's not how I think about it as seeing which versions are connecting and a server making decisions about that. For example, if I were seeing if all my clients were capable of X, this hides that ability since the library overriding these values could be anything, and using any version of SE.Redis underneath, making assessing what versions people are actually connecting with impossible.
Thoughts on the downsides there?
shacharPash commentedon Sep 3, 2023
Thanks for your consideration,
In my case I need to be able to send the lib-name&version of NRedisStack.
I don't see a particular downside here, for example, I thought of sending something like this:
then the name and version of SE.Redis and NRedisStack will be written.
mgravell commentedon Sep 3, 2023
I'm broadly OK with enabling this; if we don't folks will just bypass via Execute, so... maybe we make it easy to do well, perhaps with placeholder tokens for the default values?
shacharPash commentedon Sep 7, 2023
Hey @mgravell @NickCraver ,
Do you want me to send a PR or are you working on it?
shacharPash commentedon Sep 10, 2023
Hi, just updating that in the SETINFO command it is not possible to use
(and), therefore if we want to write both SE.Redis and the library that uses it, we will have to write it in a different way from this:NickCraver commentedon Sep 10, 2023
@shacharPash Sorry didn't see your latest there - I wanted to get RESP3 in before doing this as that whole area conflicts, but it's in now - was taking a look this morning. My plan is to separate these 2, and just outright allow overwriting LibraryVersion, and adding tokens to both name and version in a follow-up PR. Taking a peek at it now.
NickCraver commentedon Sep 10, 2023
Working this in #2547