-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
.Net: Samples, Example 14: Fix Parameters WithOpenAITextEmbeddingGenerationService #2947
Comments
PR #2946 |
This was a very small change, so I believe this PR handles it |
Does anyone have opinions on these code comments please?
instead,
instead, I recommend just removing the comment lines about indexing and embedding here. My source is the MS docs: https://learn.microsoft.com/en-us/azure/search/vector-search-how-to-generate-embeddings
|
Looping in @dmytrostruk to help with the PR review :) |
Thank you @matthewbolanos and @dmytrostruk 💪 |
I am available to help as required |
Closing as resolved in this PR: #3100. |
Describe the bug
Update Example 14 (dotnet,
KernelSyntaxExamples
) to conform to the proper use ofWithOpenAITextEmbeddingGenerationService
. The oldserviceId
parameter is no longer required here.To Reproduce
Steps to reproduce the behavior:
Example14
as instructed here: https://github.com/microsoft/semantic-kernel/tree/main/dotnet/samples/KernelSyntaxExamples#readmeNotice, the
AzureCognitiveSearchMemoryStore
example works.But then the
VolatileMemoryStore
example produces the error:HTTP 401 (Unauthorized)
Azure.RequestFailedException: Incorrect API key provided: text-emb**********-002
This error results from the modelId "text-embedding-ada-002" being passed instead of the API key.
Expected behavior
The
VolatileMemoryStore
example should work the same as the first example.We simply remove the
serviceId
parameter, as it is no longer required.Screenshots
Platform
SK-dotnet
repo (544b6c1);KernelSyntaxExamples
project.Additional context
Note, the serviceId refactor goes way back to before PR #993 (serviceregistry)
On May 1, AddOpenAITextEmbeddingGenerationService was refactored:
serviceId parameter was removed
57cbc93#diff-a95fd1663266edb1b8f7fa87865bfd8e2750bb1c7b92643255b1b39d609ac862L153
At the time of the refactor (May 1), AzureCognitiveSearchMemory did not require a TextEmbedding service
But the VolatileMemoryStorage did require a TextEmbeddingGenerationService. Back then,
serviceId
was required as the first parameter.https://github.com/microsoft/semantic-kernel/blame/5e4e0edfaa94b96f008cbd3f8669056e7a145ffa/dotnet/samples/KernelSyntaxExamples/Example14_SemanticMemory.cs
Later, TextEmbeddingGenerationSerivce was required for AzureCognitiveSearch also.
So TextEmbeddingGenerationService was added to the first example:
https://github.com/microsoft/semantic-kernel/blame/52fc85af7805d6923a4544594cf493345b9dbb7c/dotnet/samples/KernelSyntaxExamples/Example14_SemanticMemory.cs
In this same commit, the
serviceId
paramter was removed from the VolatileMemoryStorage example:https://github.com/microsoft/semantic-kernel/blame/52fc85af7805d6923a4544594cf493345b9dbb7c/dotnet/samples/KernelSyntaxExamples/Example14_SemanticMemory.cs
The
serviceId
parameter was added back to Example 14 on Jul 17. (Maybe this line was accidentally added in the merge?)THANK YOU to all the contributors at Microsoft & the community.
The text was updated successfully, but these errors were encountered: