Ref app sq lite mem store#245

Closed

amsacha wants to merge 12 commits intomicrosoft:feature-refappfrom

amsacha:RefApp_SqLiteMemStore

Contributor

amsacha commented Mar 30, 2023 •

edited

Loading

Motivation and Context

A persistent analog to Volatile Memory Store will allow for the memory concepts to persist beyond a single session and act as long term memory.
Using Sqlite as it is commonly used database engine.
Copied these internal SK classes : MinHeap.cs, Score.cs, ScoredValue.cs, TopNCollections.cs, Verify.cs
This is because we dont have access to those files as they are in another assembly.

Description

Adding a persistent memory store using Sqlite, implemented using the IMemoryStore interface.

Contribution Checklist

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

amsacha force-pushed the RefApp_SqLiteMemStore branch 2 times, most recently from 8165046 to eafb38d Compare

March 30, 2023 23:59

dluc requested changes

View reviewed changes

dotnet/src/SemanticKernel.Skills/Skills.Memory.Sqlite/Database.cs

dotnet/src/SemanticKernel.Skills/Skills.Memory.Sqlite/Imported/MinHeap.cs Outdated

samples/dotnet/KernelHttpServer/Program.cs Outdated

samples/dotnet/github-skills/GitHubSkill.cs Outdated

lemillermicrosoft added the feature label

amsacha force-pushed the RefApp_SqLiteMemStore branch 4 times, most recently from 26d9fe0 to 5799748 Compare

March 31, 2023 18:40

amsacha requested a review from dluc

March 31, 2023 18:43

adrianwyatt requested changes

View reviewed changes

dotnet/src/SemanticKernel.Skills/Skills.Memory.Sqlite/Database.cs Outdated

dotnet/src/SemanticKernel.Skills/Skills.Memory.Sqlite/Database.cs

dotnet/src/SemanticKernel.Skills/Skills.Memory.Sqlite/Database.cs Outdated

dotnet/src/SemanticKernel.Skills/Skills.Memory.Sqlite/SqliteDataStore.cs Outdated

dotnet/src/SemanticKernel.Skills/Skills.Memory.Sqlite/SqliteMemoryStore.cs Outdated

dotnet/src/SemanticKernel.Skills/Skills.Memory.Sqlite/SqliteMemoryStore.cs Outdated

dotnet/src/SemanticKernel.Skills/Skills.Memory.Sqlite/SqliteMemoryStore.cs Outdated

dotnet/src/SemanticKernel.Skills/Skills.Memory.Sqlite/SqliteMemoryStore.cs Outdated

samples/dotnet/KernelHttpServer/Program.cs Outdated

adrianwyatt added PR: feedback to address .NET labels

adrianwyatt self-assigned this

adrianwyatt removed the request for review from dluc

March 31, 2023 21:35

dluc reviewed

View reviewed changes

samples/dotnet/KernelHttpServer/Program.cs

dluc reviewed

View reviewed changes

dotnet/src/SemanticKernel.Skills/Skills.Memory.Sqlite/SqliteMemoryStore.cs Outdated

dluc reviewed

View reviewed changes

dotnet/src/SemanticKernel.Skills/Skills.Memory.Sqlite/SqliteMemoryStore.cs Outdated

dluc reviewed

View reviewed changes

dotnet/src/SemanticKernel.Skills/Skills.Memory.Sqlite/SqliteMemoryStore.cs Outdated

dluc reviewed

View reviewed changes

dotnet/src/SemanticKernel.Skills/Skills.Memory.Sqlite/Database.cs Outdated

dluc reviewed

View reviewed changes

Contributor

dluc left a comment

Please add docblocks

amsacha force-pushed the RefApp_SqLiteMemStore branch 2 times, most recently from b1dd775 to bade4b4 Compare

March 31, 2023 23:23

dluc requested changes

View reviewed changes

Contributor

dluc left a comment

I think there's some confusion on insert, updates and how conflicts are handled

amsacha requested review from adrianwyatt and dluc

April 1, 2023 00:27

amsacha force-pushed the RefApp_SqLiteMemStore branch 5 times, most recently from d514dba to 8f73b1e Compare

April 3, 2023 20:48

amsacha force-pushed the RefApp_SqLiteMemStore branch from 8f73b1e to 8a935ea Compare

April 3, 2023 21:57

amsacha added 9 commits

April 4, 2023 13:22


          initial commit; implemented memory store interface

88842de

kernelhttpserver compiles with sqliteMemoryStore;


          database builds and can perform writes, textmemoryskill.recall runnin…

6934c6c

…g into problem deserializing the interface type of IEmbeddingWithMetadata<TEmbedding>


          Deserialize string to MemoryRecord; Cleanup;

082736a

Updates to deserialization
added JsonPropertyName tags to memoryRecord

Cleanup:
remove sqliteDataStore
remove sqlite library solution
reset sqliteDataStore to state before my changes; use correct namespace
set s couple files back to their state before my changes
ran dotnet format


          reduce surface area; remove imported files; undo changes to githubski…

92832fe

…ll.cs; use UPSERT instead of INSERT for sqlite


          fixes after rebase

19e7977


          open and close sqlite connection around each operation instead of kee…

2f6e0b0

…ping it perpetually open


          added more insert variations for interacting with the sqlite database

644d2a3


          resolving PR comments

6a49a45


          Added doc blocks to a few functions

69d8c34

amsacha force-pushed the RefApp_SqLiteMemStore branch from 3a08c31 to 69d8c34 Compare

April 4, 2023 20:23


          Merge branch 'feature-refapp' into RefApp_SqLiteMemStore

025baaa

Contributor

adrianwyatt commented Apr 4, 2023 •

edited

Loading

I would like to hold this until feature-refapp comes into main this week.

amsacha and others added 2 commits

April 4, 2023 16:01


          ran dotnet format again

60aece8


          Merge branch 'feature-refapp' into RefApp_SqLiteMemStore

01a0a95

amsacha closed this

amsacha deleted the RefApp_SqLiteMemStore branch

April 11, 2023 19:53

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

.NET PR: feedback to address