Copilot commented on code in PR #7277:
URL: https://github.com/apache/ignite-3/pull/7277#discussion_r2639973589
##########
modules/platforms/dotnet/Apache.Ignite/IgniteServiceCollectionExtensions.cs:
##########
@@ -140,7 +142,19 @@ private static IServiceCollection AddIgniteClientGroupCore(
services.TryAdd(new ServiceDescriptor(
typeof(IgniteClientGroup),
key,
- (sp, innerKey) => new IgniteClientGroup(configure(sp, innerKey)),
+ (serviceProvider, innerKey) =>
+ {
+ IgniteClientGroupConfiguration cfg =
configure(serviceProvider, innerKey);
+
+ if (cfg.ClientConfiguration.LoggerFactory ==
NullLoggerFactory.Instance)
+ {
+ // Use DI logger factory if none was provided.
+ cfg.ClientConfiguration.LoggerFactory =
+ serviceProvider.GetService<ILoggerFactory>() ??
NullLoggerFactory.Instance;
Review Comment:
The code modifies the LoggerFactory property of the configuration object
that was passed in by the user. This mutates the original configuration, which
may be unexpected behavior. Consider creating a copy of the configuration
before modifying it to avoid side effects on the user's configuration object.
For example, you could use the record's with expression to create a modified
copy: `cfg = cfg with { ClientConfiguration = cfg.ClientConfiguration with {
LoggerFactory = ... } }`.
```suggestion
var loggerFactory =
serviceProvider.GetService<ILoggerFactory>() ?? NullLoggerFactory.Instance;
cfg = cfg with
{
ClientConfiguration = cfg.ClientConfiguration with
{
LoggerFactory = loggerFactory
}
};
```
##########
modules/platforms/dotnet/README.md:
##########
@@ -60,6 +60,31 @@ IJobExecution<int> jobExecution = await
client.Compute.SubmitAsync(
int wordCount = await jobExecution.GetResultAsync();
```
+## DI Integration
+
+Use `IgniteServicesCollectionExtensions.AddIgniteClientGroup` to register
Ignite client in the dependency injection container:
Review Comment:
The class name has a typo. It should be "IgniteServiceCollectionExtensions"
not "IgniteServicesCollectionExtensions" (remove the extra 's' in "Services").
```suggestion
Use `IgniteServiceCollectionExtensions.AddIgniteClientGroup` to register
Ignite client in the dependency injection container:
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
