Copilot commented on code in PR #6120:
URL: https://github.com/apache/ignite-3/pull/6120#discussion_r2166794518
##########
modules/compute/src/main/java/org/apache/ignite/internal/compute/executor/ComputeExecutorImpl.java:
##########
@@ -258,4 +260,22 @@ public void stop() {
dotNetExec.stop();
}
}
+
+ /**
+ * Handles the removal of a deployment unit.
+ *
+ * @param unitPath Path to the deployment unit that is being removed.
+ */
+ public void onUnitRemoving(Path unitPath) {
+ DotNetComputeExecutor dotNetExec = dotNetComputeExecutor;
+
+ if (dotNetExec != null) {
+ try {
+ // Do not wait for the future to complete.
+ CompletableFuture<Boolean> ignored =
dotNetExec.undeployUnitsAsync(List.of(unitPath.toRealPath().toString()));
Review Comment:
[nitpick] Consider logging or otherwise handling the outcome of the
undeployment future instead of ignoring it, to aid in diagnosing potential
issues.
```suggestion
dotNetExec.undeployUnitsAsync(List.of(unitPath.toRealPath().toString()))
.whenComplete((result, throwable) -> {
if (throwable != null) {
LOG.error("Failed to undeploy unit: " +
unitPath, throwable);
} else if (Boolean.FALSE.equals(result)) {
LOG.warn("Undeployment of unit " + unitPath + "
was unsuccessful.");
} else {
LOG.info("Successfully undeployed unit: " +
unitPath);
}
});
```
##########
modules/compute/src/main/java/org/apache/ignite/internal/compute/streamer/StreamerReceiverJob.java:
##########
@@ -46,7 +46,7 @@ public class StreamerReceiverJob implements
ComputeJob<byte[], byte[]> {
buf.slice().order(ByteOrder.LITTLE_ENDIAN),
payloadElementCount,
receiverClassName -> {
- ClassLoader classLoader = ((JobExecutionContextImpl)
context).classLoader();
+ ClassLoader classLoader = ((JobExecutionContextImpl)
context).classLoader().classLoader();
Review Comment:
[nitpick] The chained call '.classLoader().classLoader()' reduces
readability; consider assigning the inner class loader to a local variable with
a descriptive name or extracting a helper method to clarify the intent.
```suggestion
ClassLoader jobClassLoader = ((JobExecutionContextImpl)
context).classLoader();
ClassLoader classLoader = jobClassLoader.classLoader();
```
##########
modules/compute/src/main/java/org/apache/ignite/internal/compute/ComputeComponentImpl.java:
##########
@@ -354,7 +354,7 @@ private <I, M, T, R> TaskExecutionInternal<I, M, T, R>
execTask(
I input
) {
try {
- return executor.executeTask(jobSubmitter,
taskClass(context.classLoader(), taskClassName), input);
+ return executor.executeTask(jobSubmitter,
taskClass(context.classLoader().classLoader(), taskClassName), input);
Review Comment:
[nitpick] Similar to the previous note, the nested
'.classLoader().classLoader()' call affects clarity; consider abstracting the
inner class loader retrieval into a helper method with a clear name.
```suggestion
return executor.executeTask(jobSubmitter,
taskClass(getParentClassLoader(context), taskClassName), input);
```
--
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
