-
Notifications
You must be signed in to change notification settings - Fork 57
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 StoreKey to be rehydrated #114
Comments
What would be needed to make this possible? My main concern is breaking outer facing stuff, so if it doesn't I'd be inclined to go for it :) |
I don’t think removing the second dictionary will break outer facing stuff, but it’s not something I’d be looking at for this release.
In terms of what needs serializing, we need to serialize both the key/value in a key/value pair to the final string, currently only the values are serialized and joined.
The only other alternative is to have a type that derives from KeyStore for the distributed stores and do the serialization required there.
I’m open to either approach.
From: Kevin Dockx ***@***.***>
Sent: Wednesday, February 15, 2023 8:07 AM
To: KevinDockx/HttpCacheHeaders ***@***.***>
Cc: Sean Farrow ***@***.***>; Author ***@***.***>
Subject: Re: [KevinDockx/HttpCacheHeaders] Allow StoreKey to be rehydrated (Issue #114)
What would be needed to make this possible? My main concern is breaking outer facing stuff, so if it doesn't I'd be inclined to go for it :)
—
Reply to this email directly, view it on GitHub<#114 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AALDK7VJC34WV64QRAUBWKTWXSFC3ANCNFSM6AAAAAAU34YZ3A>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
Serializing both is ok as far as I'm concerned, gives us a unified approach. Do you need me to do that, or will you take this up? |
It depends when you want to get v7 out, I’m not going to get to this before Tuesday as we’re just doing a release.
I’m happy to do it though if your not in a rush as I’m blocked without it really!
Thanks
From: Kevin Dockx ***@***.***>
Sent: Friday, February 17, 2023 7:33 AM
To: KevinDockx/HttpCacheHeaders ***@***.***>
Cc: Sean Farrow ***@***.***>; Author ***@***.***>
Subject: Re: [KevinDockx/HttpCacheHeaders] Allow StoreKey to be rehydrated (Issue #114)
Serializing both is ok as far as I'm concerned, gives us a unified approach. Do you need me to do that, or will you take this up?
—
Reply to this email directly, view it on GitHub<#114 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AALDK7RIEOHZB7K7I6SSZSTWX4SUNANCNFSM6AAAAAAU34YZ3A>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
There's no rush :) |
OK, I’ll stick this on my list.
From: Kevin Dockx ***@***.***>
Sent: Friday, February 17, 2023 7:41 AM
To: KevinDockx/HttpCacheHeaders ***@***.***>
Cc: Sean Farrow ***@***.***>; Author ***@***.***>
Subject: Re: [KevinDockx/HttpCacheHeaders] Allow StoreKey to be rehydrated (Issue #114)
There's no rush :)
—
Reply to this email directly, view it on GitHub<#114 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AALDK7VYGOYQA6QG2ZNYJVTWX4TQTANCNFSM6AAAAAAU34YZ3A>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
Hi Sean, do you happen to have an idea of when you'll be able to work on this PR? Thanks! :) |
I have some holiday after the 05/05, so I’m hoping to work on it then.
Thanks,
Sean.
From: Kevin Dockx ***@***.***>
Sent: Monday, April 24, 2023 1:14 PM
To: KevinDockx/HttpCacheHeaders ***@***.***>
Cc: Sean Farrow ***@***.***>; Author ***@***.***>
Subject: Re: [KevinDockx/HttpCacheHeaders] Allow StoreKey to be rehydrated (Issue #114)
Hi Sean, do you happen to have an idea of when you'll be able to work on this PR? Thanks! :)
—
Reply to this email directly, view it on GitHub<#114 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AALDK7VRSYJTF3C4S7G2CITXCZVBNANCNFSM6AAAAAAU34YZ3A>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
That sounds good, thanks! |
@KevinDockx Do we want to spin up a Docker container for Redis in the tests to check that we can retrieve the keys a user would expect given the pattern we are matching and whether we are ignoring case? I ask as to retrieve the keys, we have to look at each server, which unfortunately doesn't have an interface, so this isn't really mockable. |
If you wish, sure, but I wouldn't go that far - seems like a bit of overkill just to get a few additional automated tests. |
I've now raised a PR for this. Once this is merged, I can finish off #112. |
Closed by #115 |
Currently, it is not possible to rehydrate a StoreKey in it's entirity as we are only creating a string witht the values in.
We need this for issue #112. As well as this, it may mean we can remove the use of a second dictionary in the in-memory implementation of IValueStore in the future.
The text was updated successfully, but these errors were encountered: