.Net: Weaviate .NET Connector: Added ReadOnlyMemoryConverter to fix deserialization#2777
Conversation
dotnet/src/Connectors/Connectors.Memory.Weaviate/WeaviateMemoryStore.cs
Outdated
Show resolved
Hide resolved
|
@jevgenigeurtsen @rogerbarreto I updated the documentation how to test Weaviate manually with local Weaviate server. I exposed After my changes I verified that integration tests are passing successfully now. Please check my latest changes and let me know if it looks good to you. Thanks! |
|
@microsoft-github-policy-service agree |
Looks good to me, nice. Can we maybe bump the version of the Weaviate container to the latest version? This woud be version Also (though this might be a different issue/PR): it would be nice if the CI runs the .NET integration tests, like it does for Python. This would've raised the issue regarding deserializing in a much earlier phase. I'm not that familiar with CI/CD on GitHub, but I'd be willing to contribute to this if needed. |
Done, I've updated the version and verified using integration tests.
We are going to work on it in the nearest future. Most probably this will require authorized access to configure GitHub Actions. |
rogerbarreto
left a comment
There was a problem hiding this comment.
Thank you both @dmytrostruk for @jevgenigeurtsen for looking into this!
…eserialization (microsoft#2777) ### Motivation and Context <!-- Thank you for your contribution to the semantic-kernel repo! Please help reviewers and future users, providing the following information: 1. Why is this change required? 3. What problem does it solve? 4. What scenario does it contribute to? 5. If it fixes an open issue, please link to the issue here. --> All operations inside `WeaviateMemoryStore` whose response models contain any property of the type `ReadOnlyMemory<>`, will throw an exception whenever the JSON deserializer tries to deserialize that specific property. I believe this is caused by the changes made in microsoft#2419 ### Description I encountered the following exception while trying out the Weaviate connector: ``` System.InvalidOperationException : The type 'System.ReadOnlySpan`1[System.Single]' of property 'Span' on type 'System.ReadOnlyMemory`1[System.Single]' is invalid for serialization or deserialization because it is a pointer type, is a ref struct, or contains generic parameters that have not been replaced by specific types. Stack Trace: at System.Text.Json.ThrowHelper.ThrowInvalidOperationException_CannotSerializeInvalidType(Type typeToConvert, Type declaringType, MemberInfo memberInfo) at System.Text.Json.Serialization.Metadata.ReflectionJsonTypeInfo`1.CreateProperty(Type typeToConvert, MemberInfo memberInfo, JsonSerializerOptions options, Boolean shouldCheckForRequiredKeyword) at System.Text.Json.Serialization.Metadata.ReflectionJsonTypeInfo`1.CacheMember(Type typeToConvert, MemberInfo memberInfo, Boolean& propertyOrderSpecified, Dictionary`2& ignoredMembers, Boolean shouldCheckForRequiredKeyword) at System.Text.Json.Serialization.Metadata.ReflectionJsonTypeInfo`1.LateAddProperties() at System.Text.Json.Serialization.Metadata.JsonTypeInfo.InitializePropertyCache() at System.Text.Json.Serialization.Metadata.JsonTypeInfo.Configure() at System.Text.Json.Serialization.Metadata.JsonTypeInfo.<EnsureConfigured>g__ConfigureLocked|143_0() at System.Text.Json.Serialization.Metadata.JsonPropertyInfo.get_JsonTypeInfo() at System.Text.Json.ReadStack.Push() at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value) at System.Text.Json.Serialization.Metadata.JsonPropertyInfo`1.ReadJsonAndSetMember(Object obj, ReadStack& state, Utf8JsonReader& reader) at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value) at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value) at System.Text.Json.Serialization.JsonCollectionConverter`2.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, TCollection& value) at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value) at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state) at System.Text.Json.JsonSerializer.ReadFromSpan[TValue](ReadOnlySpan`1 utf8Json, JsonTypeInfo jsonTypeInfo, Nullable`1 actualByteCount) at System.Text.Json.JsonSerializer.ReadFromSpan[TValue](ReadOnlySpan`1 json, JsonTypeInfo jsonTypeInfo) at System.Text.Json.JsonSerializer.Deserialize[TValue](String json, JsonSerializerOptions options) at Microsoft.SemanticKernel.Connectors.Memory.Weaviate.WeaviateMemoryStore.UpsertBatchAsync(String collectionName, IEnumerable`1 records, CancellationToken cancellationToken)+MoveNext() in C:\dotnet\src\Connectors\Connectors.Memory.Weaviate\WeaviateMemoryStore.cs:line 266 ``` The `WeaviateMemoryStore` statically instantiates a `JsonSerializerOptions` object, but without `ReadOnlyMemoryConverter`, which is required to (de)serialize properties of the `ReadOnlyMemory` type < .NET 8 (according to the comments inside the converter class). The issue only occurs whilst deserializing. Serialization works properly because the responsibility of serialization is abstracted away into `Microsoft.SemanticKernel.Connectors.Memory.Weaviate.Http.HttpRequest.CreateSerializerOptions()` -- which does indeed instantiate and passes an instance of `ReadOnlyMemoryConverter` into the `JsonSerializerOptions`. <!-- Describe your changes, the overall approach, the underlying design. These notes will help understanding how your code works. Thanks! --> ### Contribution Checklist <!-- Before submitting this PR, please make sure: --> - [X] The code builds clean without any errors or warnings - [X] The PR follows the [SK Contribution Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md) and the [pre-submission formatting script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts) raises no violations - [X] All unit tests pass, and I have added new tests where possible - [X] I didn't break anyone 😄 --------- Co-authored-by: Roger Barreto <19890735+RogerBarreto@users.noreply.github.com> Co-authored-by: Dmytro Struk <13853051+dmytrostruk@users.noreply.github.com>


Motivation and Context
All operations inside
WeaviateMemoryStorewhose response models contain any property of the typeReadOnlyMemory<>, will throw an exception whenever the JSON deserializer tries to deserialize that specific property.I believe this is caused by the changes made in #2419
Description
I encountered the following exception while trying out the Weaviate connector:
The
WeaviateMemoryStorestatically instantiates aJsonSerializerOptionsobject, but withoutReadOnlyMemoryConverter, which is required to (de)serialize properties of theReadOnlyMemorytype < .NET 8 (according to the comments inside the converter class).The issue only occurs whilst deserializing. Serialization works properly because the responsibility of serialization is abstracted away into
Microsoft.SemanticKernel.Connectors.Memory.Weaviate.Http.HttpRequest.CreateSerializerOptions()-- which does indeed instantiate and passes an instance ofReadOnlyMemoryConverterinto theJsonSerializerOptions.Contribution Checklist