Skip to content

Commit

Permalink
Fix from code review
Browse files Browse the repository at this point in the history
  • Loading branch information
kbeaugrand committed Jun 19, 2024
1 parent 4b26056 commit 1cdd569
Show file tree
Hide file tree
Showing 5 changed files with 9 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@ namespace Microsoft.SemanticKernel.Connectors.SqlServer.Classic.Core;
Justification = "We need to build the full table name using schema and collection, it does not support parameterized passing.")]
internal sealed class SqlServerClient
{
private readonly SqlServerConfig _configuration;
private readonly SqlServerClassicConfig _configuration;
private readonly SqlConnection _connection;

/// <summary>
/// Initializes a new instance of the <see cref="SqlServerClient"/> class with the specified connection string and schema.
/// </summary>
/// <param name="connection">The connection to the SQL Server database.</param>
/// <param name="configuration">The configuration to use for the SQL Server database.</param>
public SqlServerClient(SqlConnection connection, SqlServerConfig configuration)
public SqlServerClient(SqlConnection connection, SqlServerClassicConfig configuration)
{
this._configuration = configuration;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ namespace Microsoft.SemanticKernel.Connectors.SqlServer.Classic;
/// <summary>
/// Configuration for the SQL Server memory store.
/// </summary>
public class SqlServerConfig
public class SqlServerClassicConfig
{
/// <summary>
/// The default SQL Server memories table name.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ public static class SqlServerMemoryBuilderExtensions
/// <param name="connectionString">Database connection string.</param>
/// <param name="config">Configuration for the SQL Server connector.</param>
/// <returns>Updated Memory builder including SQL Server memory connector.</returns>
public static MemoryBuilder WithSqlServerMemoryStore(
public static MemoryBuilder WithSqlServerClassicMemoryStore(
this MemoryBuilder builder,
string connectionString,
SqlServerConfig? config)
SqlServerClassicConfig? config)
{
return builder.WithMemoryStore<SqlServerMemoryStore>(_ => new SqlServerMemoryStore(connectionString, config));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,9 @@ public class SqlServerMemoryStore : IMemoryStore, IDisposable
/// </summary>
/// <param name="connectionString">The connection string.</param>
/// <param name="config">The SQL memoryDB configuration.</param>
public SqlServerMemoryStore(string connectionString, SqlServerConfig? config = default)
: this(new SqlServerClient(new SqlConnection(connectionString), config ?? new()))
public SqlServerMemoryStore(string connectionString, SqlServerClassicConfig? config = default)
{
}

/// <summary>
/// Initializes a new instance of the <see cref="SqlServerMemoryStore"/> class.
/// </summary>
/// <param name="dbClient">The SQL db client.</param>
internal SqlServerMemoryStore(SqlServerClient dbClient)
{
this._dbClient = dbClient;
this._dbClient = new SqlServerClient(new SqlConnection(connectionString), config ?? new());
}

/// <inheritdoc/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public async Task InitializeAsync()
}

this._connectionString = connectionString;
this._config = new SqlServerConfig
this._config = new SqlServerClassicConfig
{
Schema = "sk_it",
CollectionTableNamePrefix = "SKMemories",
Expand Down Expand Up @@ -620,7 +620,7 @@ public async Task ItCanBatchGetRecordsAndSkipIfKeysDoNotExistAsync()
#region private ================================================================================

private string _connectionString = null!;
private SqlServerConfig _config = null!;
private SqlServerClassicConfig _config = null!;

private SqlServerMemoryStore CreateMemoryStore()
{
Expand Down

0 comments on commit 1cdd569

Please sign in to comment.