imbajin commented on code in PR #3060:
URL: https://github.com/apache/hugegraph/pull/3060#discussion_r3430871241
##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/TaskCoreTest.java:
##########
@@ -115,6 +116,79 @@ public void testTask() throws TimeoutException {
});
}
+ @Test
+ public void testTaskWithoutResult() throws TimeoutException {
+ HugeGraph graph = graph();
+ TaskScheduler scheduler = graph.taskScheduler();
+ CountDownLatch latch = new CountDownLatch(1);
+
+ TaskCallable<String> callable = new TaskCallable<String>() {
Review Comment:
⚠️ **Use a reloadable task callable in this regression test**
This anonymous `TaskCallable` is persisted by class name, then the worker
reloads it through `TaskCallable.fromClass()`. It has no public no-arg
constructor, so the task fails to load as `TaskCallable$1`, and
`scheduler.waitUntilTaskCompleted(id, 10)` times out. I reproduced it with `mvn
test -pl hugegraph-server/hugegraph-test -am -P unit-test
-Dtest=TaskCoreTest#testTaskWithoutResult -DfailIfNoTests=false -ntp`. Please
use an existing reloadable callable or add a small static callable class for
this test.
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/TaskAndResultScheduler.java:
##########
@@ -103,41 +103,65 @@ public <V> void save(HugeTask<V> task) {
}
@Override
- public <V> HugeTask<V> task(Id id) {
+ public <V> HugeTask<V> task(Id id, boolean withResult) {
HugeTask<V> task = this.call(() -> {
Iterator<Vertex> vertices = this.tx().queryTaskInfos(id);
Vertex vertex = QueryResults.one(vertices);
if (vertex == null) {
return null;
}
- return HugeTask.fromVertex(vertex);
+ return HugeTask.fromVertex(vertex, withResult);
});
if (task == null) {
throw new NotFoundException("Can't find task with id '%s'", id);
}
- HugeTaskResult taskResult = queryTaskResult(id);
- if (taskResult != null) {
- task.result(taskResult);
+ if (withResult) {
+ HugeTaskResult taskResult = queryTaskResult(id);
+ if (taskResult != null) {
+ task.result(taskResult);
+ }
}
return task;
}
@Override
public <V> Iterator<HugeTask<V>> tasks(List<Id> ids) {
- return this.tasksWithoutResult(ids);
+ return this.tasks(ids, false);
Review Comment:
‼️ **Preserve the existing `tasks(...)` result contract**
The new `TaskScheduler` default methods keep the old behavior as `tasks(ids)
-> tasks(ids, true)`, but this override changes the task/result-backed
scheduler to return metadata-only tasks by default. Existing Java callers of
`scheduler.tasks(...).next().result()` can now silently get `null` while
`scheduler.task(id)` still returns the result. Please keep the no-result path
opt-in by removing these overrides or delegating them to `withResult=true`, and
pass `false` only from the metadata-only call sites such as REST list, restore,
delete, and scheduler scans.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
