Copilot commented on code in PR #5800:
URL: https://github.com/apache/ignite-3/pull/5800#discussion_r2086437357
##########
modules/platforms/dotnet/Apache.Ignite/Internal/Compute/Executor/ComputeJobExecutor.cs:
##########
@@ -81,15 +82,24 @@ private static async ValueTask ExecuteJobAsync(
JobExecuteRequest req,
PooledBuffer argBuf,
PooledArrayBuffer resBuf,
- IJobExecutionContext context)
+ IgniteApiAccessor context)
{
// Unload assemblies after job execution.
// TODO IGNITE-25257 Cache deployment units and JobLoadContext - see
ComputeJobExecutorBenchmarks, expensive.
using JobLoadContext jobLoadCtx =
DeploymentUnitLoader.GetJobLoadContext(req.DeploymentUnitPaths);
- IComputeJobWrapper jobWrapper =
jobLoadCtx.CreateJobWrapper(req.JobClassName);
resBuf.MessageWriter.Write(0); // Response flags: success.
+ if (req.JobClassName ==
"Apache.Ignite.Internal.Table.StreamerReceiverJob, Apache.Ignite")
Review Comment:
[nitpick] Consider defining a constant for the StreamerReceiverJob class
name to avoid hardcoding the string literal, which can prevent typos and
enhance maintainability.
```suggestion
if (req.JobClassName == StreamerReceiverJobClassName)
```
##########
modules/platforms/dotnet/Apache.Ignite/Internal/Compute/Executor/JobLoadContext.cs:
##########
@@ -34,40 +36,50 @@ internal readonly record struct
JobLoadContext(AssemblyLoadContext AssemblyLoadC
/// <param name="typeName">Job type name.</param>
/// <returns>Job execution delegate.</returns>
public IComputeJobWrapper CreateJobWrapper(string typeName) =>
- CreateJobWrapper(typeName, AssemblyLoadContext);
+ CreateWrapper<IComputeJobWrapper>(
+ typeName, typeof(IComputeJob<,>), typeof(ComputeJobWrapper<,,>),
AssemblyLoadContext);
+
+ /// <summary>
+ /// Gets or creates a receiver delegate for the specified type name.
+ /// </summary>
+ /// <param name="typeName">Receiver type name.</param>
+ /// <returns>Receiver execution delegate.</returns>
+ public IDataStreamerReceiverWrapper CreateReceiverWrapper(string typeName)
=>
+ CreateWrapper<IDataStreamerReceiverWrapper>(
+ typeName, typeof(IDataStreamerReceiver<,,>),
typeof(DataStreamerReceiverWrapper<,,,>), AssemblyLoadContext);
/// <inheritdoc/>
public void Dispose() => AssemblyLoadContext.Unload();
- private static IComputeJobWrapper CreateJobWrapper(string typeName,
AssemblyLoadContext ctx)
+ private static T CreateWrapper<T>(string wrappedTypeName, Type
openInterfaceType, Type openWrapperType, AssemblyLoadContext ctx)
{
- var jobType = LoadJobType(typeName, ctx);
- var jobInterface = FindJobInterface(typeName, jobType);
+ var type = LoadType(wrappedTypeName, ctx);
+ var closedInterfaceType = FindInterface(type, openInterfaceType);
try
{
- var genericArgs = jobInterface.GenericTypeArguments;
- var jobWrapperType =
typeof(ComputeJobWrapper<,,>).MakeGenericType(jobType, genericArgs[0],
genericArgs[1]);
+ var genericArgs = closedInterfaceType.GenericTypeArguments;
Review Comment:
[nitpick] Consider adding an inline comment to explain the use of the new
spread element syntax ([type, .. genericArgs]) in MakeGenericType to help
maintainers who may be unfamiliar with the latest C# 12 patterns.
```suggestion
var genericArgs = closedInterfaceType.GenericTypeArguments;
// Use the new C# 12 spread element syntax ([type,
..genericArgs]) to combine the type
// with its generic arguments when creating the closed generic
type.
```
--
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: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
