valepakh commented on code in PR #6600:
URL: https://github.com/apache/ignite-3/pull/6600#discussion_r2352737407
##########
modules/compute/src/main/java/org/apache/ignite/internal/compute/JobExecutionContextImpl.java:
##########
@@ -44,7 +52,11 @@ public class JobExecutionContextImpl implements
JobExecutionContext {
* @param classLoader Job class loader.
* @param partition Partition associated with this job.
*/
- public JobExecutionContextImpl(Ignite ignite, AtomicBoolean isInterrupted,
JobClassLoader classLoader, @Nullable Partition partition) {
+ public JobExecutionContextImpl(
+ Ignite ignite,
+ AtomicBoolean isInterrupted,
+ JobClassLoader classLoader,
+ @Nullable Partition partition) {
Review Comment:
What's the point of this change?
##########
modules/compute/src/main/java/org/apache/ignite/internal/compute/JobExecutionContextImpl.java:
##########
@@ -74,4 +95,19 @@ public boolean isCancelled() {
public JobClassLoader classLoader() {
return classLoader;
}
+
+ private Collection<DeploymentUnitInfo> initDeploymentUnits() {
+ List<DisposableDeploymentUnit> units = classLoader.units();
+ ArrayList<DeploymentUnitInfo> result = new ArrayList<>(units.size());
+
+ try {
+ for (DisposableDeploymentUnit unit : units) {
+ result.add(new DeploymentUnitInfo(unit.unit().name(),
unit.unit().version(), unit.path().toRealPath()));
+ }
+ } catch (IOException e) {
+ throw new RuntimeException(e);
Review Comment:
`RuntimeException` doesn't look like a good exception thrown from the API
method.
##########
modules/compute/src/main/java/org/apache/ignite/internal/compute/executor/platform/PlatformComputeConnection.java:
##########
@@ -30,15 +31,14 @@ public interface PlatformComputeConnection {
* Executes a job asynchronously.
*
* @param jobId Job id (for cancellation).
- * @param deploymentUnitPaths Paths to deployment units.
* @param jobClassName Name of the job class.
* @param arg Arguments for the job.
* @return A CompletableFuture that will be completed with the result of
the job execution.
*/
CompletableFuture<ComputeJobDataHolder> executeJobAsync(
long jobId,
- List<String> deploymentUnitPaths,
String jobClassName,
+ JobExecutionContext ctx,
Review Comment:
Could you please add this parameter to javadoc?
##########
modules/compute/src/main/java/org/apache/ignite/internal/compute/JobExecutionContextImpl.java:
##########
@@ -36,6 +42,8 @@ public class JobExecutionContextImpl implements
JobExecutionContext {
private final @Nullable Partition partition;
+ private Collection<DeploymentUnitInfo> deploymentUnits;
Review Comment:
Why did you think that this comment is invalid?
##########
modules/compute/src/integrationTest/java/org/apache/ignite/internal/compute/ItComputeTestStandalone.java:
##########
@@ -222,6 +224,31 @@ void executeJobWithObsoleteUnit() {
assertThat(successJob, willCompleteSuccessfully());
}
+ @Test
+ void jobContextProvidesDeploymentUnitInfo() {
+ var units0 = List.of(
+ new DeploymentUnit(unit.name(), Version.LATEST),
+ unit);
+
+ JobDescriptor<Void, String> job =
JobDescriptor.builder(DeploymentUnitInfoJob.class).units(units0).build();
+
+ String jobRes = compute().execute(JobTarget.node(clusterNode(0)), job,
null);
+
+ assertThat(jobRes, containsString("name='jobs'"));
Review Comment:
Can't we just return the `DeploymentUnitInfo` from the job and compare it
with the expected given that this is embedded node and local execution?
--
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]
