kl0u commented on a change in pull request #13911:
URL: https://github.com/apache/flink/pull/13911#discussion_r518602324
##########
File path:
flink-clients/src/main/java/org/apache/flink/client/deployment/application/EmbeddedJobClient.java
##########
@@ -126,7 +126,8 @@ public JobID getJobID() {
try {
return
jobResult.toJobExecutionResult(classLoader);
} catch (Throwable t) {
- throw new
CompletionException(new Exception("Job " + jobId + " failed", t));
+ throw new CompletionException(
+
UnsuccessfulTerminationException.fromJobResult(jobResult, classLoader));
Review comment:
For the first part, for now this is not the contract. Other
implementations of the `JobClient` do not throw this exception and I did not
like to change it in case people have exception handlers based on the current
behaviour. Currently the `ClusterClientJobClientAdapter` wraps the exception
from the `JobResult.toJobExecutionResult()` into a `ProgramInvocationException`
and the `MiniClusterJobClient` simply wraps it in a `CompletionException`. So I
would propose making this uniform and part of the contract in another issue.
That said, I believe that an exception with the job status like the
`UnsuccessfulExecutionException` is a good candidate for the common behaviour.
For the second part, yes the `ApplicationMode` only works with an
`EmbeddedJobClient`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
