semantic-kernel icon indicating copy to clipboard operation
semantic-kernel copied to clipboard

Redis connector for Semantic Memory

Open JadynWong opened this issue 1 year ago • 3 comments

Motivation and Context

Redis supports cosine similarity search by the RediSearch module.

Description

Add a new Semantic Memory Connector leveraging Redis with RediSearch module. Update the syntax example to demonstrate how it is used.

Contribution Checklist

  • [x] The code builds clean without any errors or warnings
  • [x] The PR follows SK Contribution Guidelines (https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
  • [x] The code follows the .NET coding conventions (https://learn.microsoft.com/dotnet/csharp/fundamentals/coding-style/coding-conventions) verified with dotnet format
  • [x] All unit tests pass, and I have added new tests where possible
  • [x] I didn't break anyone :smile:

Close: #1177

JadynWong avatar May 23 '23 10:05 JadynWong

Hello @lemillermicrosoft, Would you be willing to review this PR, please? Thank you so much for your time!

JadynWong avatar May 23 '23 11:05 JadynWong

@JadynWong

There's a conflict over pgvector versioning due to PostgresMemoryStore depending on 0.1.2 and this depending on 0.1.3. I don't have a preference of one over the other unless up/downgrading the version breaks one of the memory stores.

awharrison-28 avatar May 23 '23 22:05 awharrison-28

@JadynWong

There's a conflict over pgvector versioning due to PostgresMemoryStore depending on 0.1.2 and this depending on 0.1.3. I don't have a preference of one over the other unless up/downgrading the version breaks one of the memory stores.

Hi @awharrison-28,

I merged the latest main branch and fixed the conflict. I have tested them and they work fine. Thanks.

JadynWong avatar May 24 '23 00:05 JadynWong

This is great to see. Thank you @JadynWong! @awharrison-28 will help verify and approve this change.

shawncal avatar Jun 01 '23 09:06 shawncal

Hi @awharrison-28, it's been a while, is there anything that needs to be changed? Thank you.

JadynWong avatar Jun 08 '23 10:06 JadynWong

Hi @dmytrostruk, The workflows exception does not seem to be caused by the current PR changing the content, does it affect the merge?

JadynWong avatar Jun 09 '23 10:06 JadynWong

Hi @dmytrostruk, The workflows exception does not seem to be caused by the current PR changing the content, does it affect the merge?

Hi @JadynWong, something wrong with pipelines, we will fix that. As far as I know, these are not blocking to merge. We expect to merge this PR to main today 😃

dmytrostruk avatar Jun 09 '23 10:06 dmytrostruk