Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Weaviate connector #1219

Merged
merged 41 commits into from
Jun 8, 2023
Merged

Weaviate connector #1219

merged 41 commits into from
Jun 8, 2023

Conversation

codebrain
Copy link
Contributor

Motivation and Context

Description

This adds a WeaviateMemoryStore, which allows for memories to be stored in Weaviate.

I have some outstanding questions, please help!

  1. Weaviate has strict naming conventions around class names (i.e. collections). What are the conventions in SK for class names?
  2. What are the most appropriate log levels for the connector? Please can you verify these are as expected?
  3. I have a suite of integration tests that can run against a Weaviate instance. I have implemented these integration tests before, in another project. Any guidance on how these should run for this repo? Ultimately it requires Weaviate running in a docker container.

Contribution Checklist

@github-actions github-actions bot added .NET Issue or Pull requests regarding .NET code kernel Issues or pull requests impacting the core kernel samples labels May 25, 2023
@codebrain
Copy link
Contributor Author

codebrain commented May 25, 2023 •
edited
Loading

Recreated from: #738

Although I have now discovered, it is not possible to allow edits by maintainers, since the fork exists with in organisation.

@codebrain
Copy link
Contributor Author

Ping: @lemillermicrosoft

lemillermicrosoft
lemillermicrosoft reviewed May 25, 2023
View reviewed changes
Copy link
Member

@lemillermicrosoft lemillermicrosoft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still working on running this locally. Also see a handful of warnings that I haven't noted in the PR yet. Leaving comments for now. Thanks!

samples/apps/copilot-chat-app/webapi/README.md Outdated Show resolved Hide resolved
dotnet/src/IntegrationTests/Connectors/Weaviate/WeaviateMemoryStoreTests.cs Outdated Show resolved Hide resolved
dotnet/src/Connectors/Connectors.Memory.Weaviate/Diagnostics/NullableAttributes.cs Show resolved Hide resolved
dotnet/src/Connectors/Connectors.Memory.Weaviate/Diagnostics/Verify.cs Show resolved Hide resolved
dotnet/src/Connectors/Connectors.Memory.Weaviate/Http/ApiSchema/GraphGetRequest.cs Outdated Show resolved Hide resolved
samples/apps/copilot-chat-app/webapi/SemanticKernelExtensions.cs Outdated Show resolved Hide resolved
samples/dotnet/kernel-syntax-examples/Example41_Weaviate.cs Outdated Show resolved Hide resolved
samples/dotnet/kernel-syntax-examples/Example41_Weaviate.cs Outdated Show resolved Hide resolved
samples/apps/copilot-chat-app/webapi/README.md Outdated Show resolved Hide resolved
samples/dotnet/kernel-syntax-examples/Example41_Weaviate.cs Outdated Show resolved Hide resolved
@lemillermicrosoft lemillermicrosoft added the PR: feedback to address Waiting for PR owner to address comments/questions label May 25, 2023
codebrain and others added 11 commits May 30, 2023 16:16
@lemillermicrosoft
Copy link
Member

Still see some formatting issues and build issues. Waiting on that for further testing.

@codebrain
Copy link
Contributor Author

codebrain commented Jun 1, 2023 •
edited
Loading

Still see some formatting issues and build issues. Waiting on that for further testing.

Is there anything specifically I can do to have this merged

  • I have opened and compiled in VS and fixed the trailing whitespaces (see commit below).
  • I don't believe the build errors in the solution are as a result of the changes I have made, the errors are in files I have not changed, and in the file I did change I simply copy and pasted most of the implementation from the Qdrant connector.
    image

@lemillermicrosoft lemillermicrosoft enabled auto-merge (squash) June 6, 2023 16:27
@lemillermicrosoft lemillermicrosoft requested review from dluc and removed request for dluc June 6, 2023 16:28
@dluc dluc dismissed their stale review June 6, 2023 16:59

build fixed

auto-merge was automatically disabled June 8, 2023 08:07

Head branch was pushed to by a user without write access

@codebrain codebrain requested a review from dluc June 8, 2023 08:13
@codebrain
Copy link
Contributor Author

Hi @dluc - I have had to resolve conflicts again in 9dfd477
The example number needed to be bumped from 43 to 44.

When this branch goes green can it please be merged? Thanks.

@dluc
Copy link
Collaborator

dluc commented Jun 8, 2023

I'd like to merge this today but somehow github continues to report conflicts, resetting the approval flags

@lemillermicrosoft lemillermicrosoft mentioned this pull request Jun 8, 2023
Merged
@lemillermicrosoft
Copy link
Member

Hi @dluc - I have had to resolve conflicts again in 9dfd477 The example number needed to be bumped from 43 to 44.

When this branch goes green can it please be merged? Thanks.

I've opened searchpioneer#1 against searchpioneer to update this PR.

lemillermicrosoft and others added 7 commits June 8, 2023 12:55
@lemillermicrosoft
🗑️ Remove Weaviate memory store option
239cfca 
This commit removes the option to use Weaviate as a persistent memory
store for the Copilot Chat API service. Weaviate is no longer supported
as a vector database for the semantic kernel. The commit deletes the
WeaviateOptions class and the corresponding configuration settings, and
updates the README file to remove the instructions for enabling the
Weaviate memory store. The Qdrant and Azure Cognitive Search memory
stores are still available as alternatives.
@lemillermicrosoft
🐛 Fix wrong example number for Weaviate
81412f4 
The example number for Weaviate was 44 instead of 46, which was
inconsistent with the file name and the order of the examples. This
commit fixes the typo and updates the example number to 46. No other
changes were made to the code or the logic.
@lemillermicrosoft
merge main
cd917e7
@lemillermicrosoft
Squashed commit of the following:
6aec90a 
commit d3775b0
Author: Matt Mazzola <[email protected]>
Date:   Thu Jun 8 12:43:17 2023 -0700

    style: chat-summary-webapp-react - change label for deployment form input from Model to Deployment (microsoft#1368)

    There are different sets of parameters when using OpenAI vs Azure OpenAI

    1. OpenAI Key
    1. Model (`text-davinci-003`)
    You can deploy multiple versions of the same model (`gpt-3.5-turbo`)
    with different resource throughput. This is why "model" is not unique
    enough. Instead of Model you specify Deployment and Endpoint

    1. OpenAI Key
    1. Deployment
    1. Endpoint

    The `samples\apps\chat-summary-webapp-react` it was using Model term
    which may be misleading

    - samples\apps\chat-summary-webapp-react
        - Change label for Deployment form input from Model to Deployment

commit 1a7639a
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Thu Jun 8 18:13:30 2023 +0000

    Bump PdfPig from 0.1.8-alpha-20230423-3898f to 0.1.8-alpha-20230529-6daa2 in /dotnet (microsoft#1267)

    Bumps [PdfPig](https://github.com/UglyToad/PdfPig) from
    0.1.8-alpha-20230423-3898f to 0.1.8-alpha-20230529-6daa2.
    <details>
    <summary>Release notes</summary>
    <p><em>Sourced from <a
    href="https://github.com/UglyToad/PdfPig/releases">PdfPig's
    releases</a>.</em></p>
    <blockquote>
    <h2>0.1.2 Second Alpha</h2>
    <p>Adds letter font details and a couple of other bugfixes to the alpha
    version.</p>
    </blockquote>
    </details>
    <details>
    <summary>Commits</summary>
    <ul>
    <li>See full diff in <a
    href="https://github.com/UglyToad/PdfPig/commits">compare view</a></li>
    </ul>
    </details>
    <br />

    [![Dependabot compatibility
    score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=PdfPig&package-manager=nuget&previous-version=0.1.8-alpha-20230423-3898f&new-version=0.1.8-alpha-20230529-6daa2)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

    Dependabot will resolve any conflicts with this PR as long as you don't
    alter it yourself. You can also trigger a rebase manually by commenting
    `@dependabot rebase`.

    [//]: # (dependabot-automerge-start)
    [//]: # (dependabot-automerge-end)

    ---

    <details>
    <summary>Dependabot commands and options</summary>
    <br />

    You can trigger Dependabot actions by commenting on this PR:
    - `@dependabot rebase` will rebase this PR
    - `@dependabot recreate` will recreate this PR, overwriting any edits
    that have been made to it
    - `@dependabot merge` will merge this PR after your CI passes on it
    - `@dependabot squash and merge` will squash and merge this PR after
    your CI passes on it
    - `@dependabot cancel merge` will cancel a previously requested merge
    and block automerging
    - `@dependabot reopen` will reopen this PR if it is closed
    - `@dependabot close` will close this PR and stop Dependabot recreating
    it. You can achieve the same result by closing it manually
    - `@dependabot ignore this major version` will close this PR and stop
    Dependabot creating any more for this major version (unless you reopen
    the PR or upgrade to it yourself)
    - `@dependabot ignore this minor version` will close this PR and stop
    Dependabot creating any more for this minor version (unless you reopen
    the PR or upgrade to it yourself)
    - `@dependabot ignore this dependency` will close this PR and stop
    Dependabot creating any more for this dependency (unless you reopen the
    PR or upgrade to it yourself)

    </details>

    ---------

    Signed-off-by: dependabot[bot] <[email protected]>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
    Co-authored-by: Adrian Bonar <[email protected]>

commit 729d9d7
Author: garethdebruyn <[email protected]>
Date:   Thu Jun 8 19:47:22 2023 +0200

    Update Example02_Pipeline.cs (microsoft#1286)

    text["LStrip"], text["RStrip"] no longer seem to be part of TextSkill

commit de74668
Author: Lee Miller <[email protected]>
Date:   Thu Jun 8 10:41:02 2023 -0700

    Fixing typos (microsoft#1377)
    Fixing typos to unblock PRs.

commit 3bc0967
Author: Roger Barreto <[email protected]>
Date:   Thu Jun 8 17:29:52 2023 +0100

    IChatCompletion Multiple Results / New Interfaces (microsoft#1117)

    Add potential for IChatCompletion with multiple results per prompt.

    ![MultiChatResultPR](https://github.com/microsoft/semantic-kernel/assets/19890735/725ecce9-5e22-4a46-a06d-b159c68807f0)

    - Adds new Interfaces/Abstractions for Chat Results: `IChatResult`,
    `IChatStreamingResult`, `IChatMessage`
    - Adds new `IChatCompletion` interface methods for Streaming and
    Non-Streaming results
    - Moves old `IChatCompletion` interface methods
    `IAsyncEnumerable<string> GenerateMessageStreamAsync` and `Task<string>
    GenerateMessageAsync` as extensions for retro compatibility
    - Changed `ChatAsText` to use the new multiple results implementations
    - Makes `ChatHistory.ChatMessage` & `ChatHistory.AuthorsRole` types
    obsolete (discouraging usage of those implementation in favor of new
    `IChatMessage`
    - `ChatHistory` abstraction now extends from `List<IChatMessage>`
    - Updates all Samples using the new interfaces.
    - Remove `ChatHistory.ChatMessage` inner class
    - Remove `ChatHistory.AuthorsRole` enums
    - Make `ChatHistory` an abstract class or an `IChatHistory` interface.
    - Rename `ITextCompletionResult` to `ITextResult`
    - Rename `ITextComplet
@lemillermicrosoft
Merge branch 'main' into weaviate-connector-sk
d510f61
@lemillermicrosoft
Merge branch 'main' into weaviate-connector-sk
4076c83
@codebrain
Merge pull request #1 from lemillermicrosoft/weaviate-connector-sk
d07efba 
Weaviate connector sk
@dluc dluc merged commit 5742bb1 into microsoft:main Jun 8, 2023
shawncal pushed a commit to shawncal/semantic-kernel that referenced this pull request Jul 6, 2023
@codebrain
Weaviate connector (microsoft#1219)
a582f36 
Adds a WeaviateMemoryStore, which allows for memories to be stored in Weaviate.
golden-aries pushed a commit to golden-aries/semantic-kernel that referenced this pull request Oct 10, 2023
@codebrain
Weaviate connector (microsoft#1219)
49eec53 
Adds a WeaviateMemoryStore, which allows for memories to be stored in Weaviate.
johnoliver pushed a commit to johnoliver/semantic-kernel that referenced this pull request Jun 5, 2024
@codebrain
Weaviate connector (microsoft#1219)
24946a1 
Adds a WeaviateMemoryStore, which allows for memories to be stored in Weaviate.
johnoliver pushed a commit to johnoliver/semantic-kernel that referenced this pull request Jun 5, 2024
@codebrain
Weaviate connector (microsoft#1219)
4d81ef0 
Adds a WeaviateMemoryStore, which allows for memories to be stored in Weaviate.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmytrostruk dmytrostruk dmytrostruk left review comments

@lemillermicrosoft lemillermicrosoft lemillermicrosoft left review comments

@dluc dluc Awaiting requested review from dluc

Assignees

@dmytrostruk dmytrostruk

Labels
kernel Issues or pull requests impacting the core kernel memory connector .NET Issue or Pull requests regarding .NET code PR: ready for review All feedback addressed, ready for reviews PR: ready to merge PR has been approved by all reviewers, and is ready to merge.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@codebrain @lemillermicrosoft @dmytrostruk @dluc @evchaki