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.

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

Temporary workaround for handling Http NotFound exception in MemoryStoreExtender #73

Merged
merged 1 commit into from
Feb 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ Previous classification is not required if changes are simple or all belong to t
- Added an example of using `KeyPhrasesLocaled` in `Encamina.Enmarcha.Samples.SemanticKernel.Text`.
- New text prompt function for translate texts, `Translate`.
- Added an example of using `Translate` in `Encamina.Enmarcha.Samples.SemanticKernel.Text`.
- Bug fix: Temporary workaround for handling Http NotFound exception in `MemoryStoreExtender`. [(#72)](https://github.com/Encamina/enmarcha/issues/72)

## [8.1.2]

Expand Down
2 changes: 1 addition & 1 deletion Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

<PropertyGroup>
<VersionPrefix>8.1.3</VersionPrefix>
<VersionSuffix>preview-03</VersionSuffix>
<VersionSuffix>preview-04</VersionSuffix>
</PropertyGroup>

<!--
Expand Down
25 changes: 21 additions & 4 deletions src/Encamina.Enmarcha.SemanticKernel/MemoryStoreExtender.cs
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
// Ignore Spelling: Upsert

using System;
using System.Collections.ObjectModel;
using System.Net;
using System.Runtime.CompilerServices;
using System.Text.Json;

using Azure;

using Encamina.Enmarcha.SemanticKernel.Abstractions;
using Encamina.Enmarcha.SemanticKernel.Abstractions.Events;

Expand Down Expand Up @@ -45,7 +47,7 @@
public void RaiseMemoryStoreEvent(MemoryStoreEventArgs e) => MemoryStoreEvent?.Invoke(this, e);

/// <inheritdoc/>
public virtual async Task UpsertMemoryAsync(string memoryId, string collectionName, IEnumerable<string> chunks, Kernel kernel, IDictionary<string, string> metadata = null, CancellationToken cancellationToken = default)

Check warning on line 50 in src/Encamina.Enmarcha.SemanticKernel/MemoryStoreExtender.cs

View workflow job for this annotation

GitHub Actions / CI

Split this 222 characters long line (which is greater than 200 authorized). (https://rules.sonarsource.com/csharp/RSPEC-103)

Check warning on line 50 in src/Encamina.Enmarcha.SemanticKernel/MemoryStoreExtender.cs

View workflow job for this annotation

GitHub Actions / CI

Split this 222 characters long line (which is greater than 200 authorized). (https://rules.sonarsource.com/csharp/RSPEC-103)

Check warning on line 50 in src/Encamina.Enmarcha.SemanticKernel/MemoryStoreExtender.cs

View workflow job for this annotation

GitHub Actions / CI

Split this 222 characters long line (which is greater than 200 authorized). (https://rules.sonarsource.com/csharp/RSPEC-103)

Check warning on line 50 in src/Encamina.Enmarcha.SemanticKernel/MemoryStoreExtender.cs

View workflow job for this annotation

GitHub Actions / CI

Split this 222 characters long line (which is greater than 200 authorized). (https://rules.sonarsource.com/csharp/RSPEC-103)
{
await DeleteMemoryAsync(memoryId, collectionName, cancellationToken);
await SaveChunks(memoryId, collectionName, chunks, metadata, kernel, cancellationToken);
Expand Down Expand Up @@ -85,7 +87,7 @@
}

/// <inheritdoc/>
public virtual async IAsyncEnumerable<string> BatchUpsertMemoriesAsync(string collectionName, IDictionary<string, MemoryContent> memoryContents, Kernel kernel, [EnumeratorCancellation] CancellationToken cancellationToken)

Check warning on line 90 in src/Encamina.Enmarcha.SemanticKernel/MemoryStoreExtender.cs

View workflow job for this annotation

GitHub Actions / CI

Split this 225 characters long line (which is greater than 200 authorized). (https://rules.sonarsource.com/csharp/RSPEC-103)

Check warning on line 90 in src/Encamina.Enmarcha.SemanticKernel/MemoryStoreExtender.cs

View workflow job for this annotation

GitHub Actions / CI

Split this 225 characters long line (which is greater than 200 authorized). (https://rules.sonarsource.com/csharp/RSPEC-103)

Check warning on line 90 in src/Encamina.Enmarcha.SemanticKernel/MemoryStoreExtender.cs

View workflow job for this annotation

GitHub Actions / CI

Split this 225 characters long line (which is greater than 200 authorized). (https://rules.sonarsource.com/csharp/RSPEC-103)

Check warning on line 90 in src/Encamina.Enmarcha.SemanticKernel/MemoryStoreExtender.cs

View workflow job for this annotation

GitHub Actions / CI

Split this 225 characters long line (which is greater than 200 authorized). (https://rules.sonarsource.com/csharp/RSPEC-103)
{
var memoryRecords = new List<MemoryRecord>();
var textEmbeddingGenerationService = kernel.GetRequiredService<ITextEmbeddingGenerationService>();
Expand Down Expand Up @@ -132,15 +134,30 @@
private async Task<int> GetChunkSize(string memoryId, string collectionName, CancellationToken cancellationToken)
{
var key = BuildMemoryIdentifier(memoryId, 0);
var fistMemoryChunk = await MemoryStore.GetAsync(collectionName, key, cancellationToken: cancellationToken);
MemoryRecord firstMemoryChunk = null;

try
{
firstMemoryChunk = await MemoryStore.GetAsync(collectionName, key, cancellationToken: cancellationToken);
}
catch (RequestFailedException e) when (e.Status == (int)HttpStatusCode.NotFound)
{
// At this point, we need to catch the NotFound exception. This is necessary because, in the case of Azure AI Search Memory Store,
// if the element does not exist, it throws an exception instead of returning a null value, which would be the expected behavior.
// We have opened an issue in Semantic Kernel and also an issue in ENMARCHA to track the progress of this issue and remove this try/catch block once it is resolved.
// Issue ENMARCHA: https://github.com/Encamina/enmarcha/issues/72

/* Do nothing */
}

RaiseMemoryStoreEvent(new() { EventType = MemoryStoreEventTypes.Get, Keys = [key], CollectionName = collectionName });

if (fistMemoryChunk == null)
if (firstMemoryChunk == null)
{
return 0;
}

var metadata = JsonSerializer.Deserialize<Dictionary<string, string>>(fistMemoryChunk.Metadata.AdditionalMetadata);
var metadata = JsonSerializer.Deserialize<Dictionary<string, string>>(firstMemoryChunk.Metadata.AdditionalMetadata);

return int.Parse(metadata[ChunkSize]);
}
Expand Down
Loading