.Net: SqlServer dependency injection extension methods #11909

adamsitnik · 2025-05-06T15:08:56Z

It's a resurrected and updated #11594.

Please keep in mind that the main point is establishing a pattern that is going to be applied to all other connectors:

we extend IServiceCollection services by creating a new static class called '{$ConnectorName}ServiceCollectionExtensions', part of the Microsoft.SemanticKernel namespace
we provide extension methods to register both the vector store and the collection: Add{$ConnectorName}VectorStore and Add{$ConnectorName}Collection. And a Keyed variant with AddKeyed prefix.
for both the vector store and the collection, we register factory methods for the concrete type and try to register them for the abstractions as well (in case user uses multiple connectors, they are all registered, but only the first one can be resolved via the abstraction)
ServiceLifetime.Singleton is the default, but users can customize it
We provide overloads that take Func which accepts IServiceProvider to obtain every setting (to be able to read from the config or resolve from the DI)
We provide convenience overloads for things that are expected to be frequently used like string connectionString

adamsitnik · 2025-05-06T15:19:44Z

dotnet/src/VectorDataIntegrationTests/VectorDataIntegrationTests/DependencyInjectionTests.cs

+
+    protected abstract void RegisterVectorStore(IServiceCollection services, ServiceLifetime lifetime, object? serviceKey = null);
+
+    protected abstract void RegisterCollection(IServiceCollection services, ServiceLifetime lifetime, string collectionName = "name", object? serviceKey = null);


@roji regarding your comment about exposing a list of delegates from https://github.com/microsoft/semantic-kernel/pull/11594/files#r2049050430, I propose to take what I have now and let me shape the final form of the base DI test class in the next PR, once I apply the same pattern to all the connectors and have the full picture

Absolutely no problem to finalize the tests later! I do think we'll ideally want to cover more than just one register method (which is what this allows), but definitely ok for now.

roji

@adamsitnik looks great! Here's a first round of feedback.

dotnet/src/Connectors/Connectors.Memory.SqlServer/SqlServerServiceCollectionExtensions.cs

dotnet/src/VectorDataIntegrationTests/VectorDataIntegrationTests/DependencyInjectionTests.cs

roji · 2025-05-06T16:11:36Z

dotnet/src/VectorDataIntegrationTests/VectorDataIntegrationTests/DependencyInjectionTests.cs

+
+    [Theory]
+    [MemberData(nameof(LiftetimesAndKeys))]
+    public virtual void CanRegisterConcreteTypeVectorStoreAfterSomeAbstractionHasBeenRegistered(ServiceLifetime lifetime, object? serviceKey)


I personally very much use snake_case (see filter tests) for test names precisely for this kind of super long test name, which I personally find really hard to read (there's a reason humans put spaces between words 🤣 ).

But definitely not blocking, we're already inconsistent on this - we'll clean up afterwards.

dotnet/src/VectorDataIntegrationTests/VectorDataIntegrationTests/DependencyInjectionTests.cs

dotnet/src/Connectors/Connectors.Memory.SqlServer/SqlServerServiceCollectionExtensions.cs

Co-authored-by: Shay Rojansky <roji@roji.org>

- move the extension methods to Microsoft.Extensions.DependencyInjection namespace as they extend IServiceCollection from this namespace - resolve IEmbeddingGenerator and set it for the option bag - use Add rather than TryAdd - add note about HybridSearch - remove the need to use Hosting - renames and simplification

…one.

…o sqlServerDIAgain

dotnet/src/Connectors/Connectors.Memory.SqlServer/SqlServerServiceCollectionExtensions.cs

- reduce code duplication - reduce XML docs duplication - create a copy of the option bag to avoid modifying user provided input

roji

See mainly comment about refactoring out the options copying logic to an internal copy constructors, otherwise LGTM!

dotnet/src/Connectors/Connectors.Memory.SqlServer/SqlServerServiceCollectionExtensions.cs

roji · 2025-05-07T15:48:39Z

dotnet/src/Connectors/Connectors.Memory.SqlServer/SqlServerServiceCollectionExtensions.cs

+    /// Registers a <see cref="SqlServerVectorStore"/> as <see cref="VectorStore"/>, with the specified connection string and service lifetime.
+    /// </summary>
+    /// <inheritdoc cref="AddVectorStore"/>
+    public static IServiceCollection AddSqlServerVectorStore(


Just a thought - would be helpful to use #region to group together methods which belong together (e.g. the keyed/non-keyed pair plus the private implementation they delegate to). Not critical.

Me using regions: https://youtu.be/J0jA89ro48w?t=201

Ahahahaha yeah I thought when I proposed this that it might be a bit... controversial ;)

…ance they are going to get updated when a new property is introduced

SqlServer dependency injection extension methods

5687706

adamsitnik requested review from roji and westey-m May 6, 2025 15:08

adamsitnik requested a review from a team as a code owner May 6, 2025 15:08

markwallace-microsoft added .NET Issue or Pull requests regarding .NET code kernel Issues or pull requests impacting the core kernel memory labels May 6, 2025

github-actions bot changed the title ~~SqlServer dependency injection extension methods~~ .Net: SqlServer dependency injection extension methods May 6, 2025

adamsitnik temporarily deployed to integration May 6, 2025 15:09 — with GitHub Actions Inactive

adamsitnik commented May 6, 2025

View reviewed changes

roji requested changes May 6, 2025

View reviewed changes

westey-m reviewed May 6, 2025

View reviewed changes

dotnet/src/Connectors/Connectors.Memory.SqlServer/SqlServerServiceCollectionExtensions.cs Outdated Show resolved Hide resolved

Apply suggestions from code review

2374a60

Co-authored-by: Shay Rojansky <roji@roji.org>

adamsitnik temporarily deployed to integration May 7, 2025 08:55 — with GitHub Actions Inactive

adamsitnik temporarily deployed to integration May 7, 2025 10:33 — with GitHub Actions Inactive

adamsitnik requested review from roji and westey-m May 7, 2025 10:38

westey-m approved these changes May 7, 2025

View reviewed changes

adamsitnik added 2 commits May 7, 2025 14:31

Resolve the embedding generator only in case the user didn't provide …

4f690d4

…one.

Merge remote-tracking branch 'upstream/feature-vector-data-preb3' int…

37d53dd

…o sqlServerDIAgain

adamsitnik temporarily deployed to integration May 7, 2025 12:41 — with GitHub Actions Inactive

roji reviewed May 7, 2025

View reviewed changes

address code review feedback:

ea8a21d

- reduce code duplication - reduce XML docs duplication - create a copy of the option bag to avoid modifying user provided input

adamsitnik temporarily deployed to integration May 7, 2025 15:33 — with GitHub Actions Inactive

roji approved these changes May 7, 2025

View reviewed changes

address code review feedback: introduce copy ctors to increase the ch…

f37f595

…ance they are going to get updated when a new property is introduced

adamsitnik temporarily deployed to integration May 7, 2025 16:10 — with GitHub Actions Inactive

adamsitnik merged commit d5d52f8 into microsoft:feature-vector-data-preb3 May 7, 2025
11 checks passed

adamsitnik mentioned this pull request May 7, 2025

[MEVD] SQL Server: implement DI registration methods #10948

Closed


		protected abstract void RegisterVectorStore(IServiceCollection services, ServiceLifetime lifetime, object? serviceKey = null);

		protected abstract void RegisterCollection(IServiceCollection services, ServiceLifetime lifetime, string collectionName = "name", object? serviceKey = null);

.Net: SqlServer dependency injection extension methods #11909

.Net: SqlServer dependency injection extension methods #11909

Uh oh!

Conversation

adamsitnik commented May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adamsitnik May 6, 2025

Choose a reason for hiding this comment

Uh oh!

roji May 6, 2025

Choose a reason for hiding this comment

Uh oh!

roji left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

roji May 6, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

roji left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

roji May 7, 2025

Choose a reason for hiding this comment

Uh oh!

adamsitnik May 7, 2025

Choose a reason for hiding this comment

Uh oh!

roji May 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

adamsitnik commented May 6, 2025 •

edited

Loading