[Java] Support calling functions returning void #5494
Conversation
|
Test PASSed. |
|
Test PASSed. |
| @@ -7,5 +7,6 @@ | |||
| */ | |||
| @FunctionalInterface | |||
| public interface RayFunc0<R> extends RayFunc { | |||
| R apply(); | |||
|
|
|||
| R apply() throws Exception; | |||
There was a problem hiding this comment.
I guess I missed something else, just wondering how does this relate to supporting void return?
There was a problem hiding this comment.
This is a small fix for another issue. Without this, Ray.call can't take a RayFunc with throws Exception declaration.
| @@ -143,15 +146,15 @@ public RayObject callPy(String moduleName, String functionName, Object[] args, | |||
| checkPyArguments(args); | |||
| PyFunctionDescriptor functionDescriptor = new PyFunctionDescriptor(moduleName, "", | |||
| functionName); | |||
| return callNormalFunction(functionDescriptor, args, options); | |||
| return callNormalFunction(functionDescriptor, args, 1, options); | |||
There was a problem hiding this comment.
maybe add a comment somewhere that void return is not supported for cross-lang calls.
There was a problem hiding this comment.
added a comment above this line.
| @@ -70,10 +70,10 @@ public TaskExecutor(AbstractRayRuntime runtime) { | |||
|
|
|||
| List<NativeRayObject> returnObjects = new ArrayList<>(); | |||
| ClassLoader oldLoader = Thread.currentThread().getContextClassLoader(); | |||
| // Find the executable object. | |||
| RayFunction rayFunction = runtime.getFunctionManager() | |||
There was a problem hiding this comment.
Is it possible that it can't find the method and results in an exception thrown here? Since you moved it out of the try-catch clause, this will make the worker process crash.
There was a problem hiding this comment.
If the method can't be found, this line will only return null, won't throw exceptions.
There was a problem hiding this comment.
OK. A NullPointerException would be thrown inside try block and caught by catch block. Then you access rayFunction again in catch block, again a new NullPointerException would be thrown.
There was a problem hiding this comment.
I see. But I think it's desirable to crash worker if rayFunction is null. because it indicates a system bug, not an app-level exception. I'll add a checkNonNull to make it more explicit.
| * @param object The object to put. | ||
| * @param objectId Object id. | ||
| */ | ||
| public void put(Object object, ObjectId objectId) { |
There was a problem hiding this comment.
Since you've added a put method for testing purposes, can you search and update test code not related to this PR? I believe there are some existing test cases which achieve the same by executing objectStore.putRaw(objectStore.serialize(object), objectId).
There was a problem hiding this comment.
updated testPutWithDuplicateId
|
Test PASSed. |
|
Test PASSed. |
|
Test PASSed. |
|
Test FAILed. |
Why are these changes needed?
What do these changes do?
return void.Ray.callcan't take aRayFuncwiththrows Exceptiondeclaration.Related issue number
Linter
scripts/format.shto lint the changes in this PR.