TisonKun commented on issue #9972: [FLINK-14496][client] Exclude detach flag from ClusterClient URL: https://github.com/apache/flink/pull/9972#issuecomment-545368487 Thanks for your review @aljoscha ! 1. For `submit(...).thenApply(...).thenCompose(...)` I don't think they are duplicate code because 1). if one want to block until submission succeed, he calls `submit(...).get()` 2). if one want to block until job result delivered, he calls `submit(...).thenCompose(client::requestJobResult).get())`; note that `thenApply(...)` is an unfortunately redundant because we return `JobSubmissionResult` as a redundant wrapper of `JobID`. 3). if one want to deserialize `JobResult` as `JobExecutionResult`, mainly deserialize accumulator, he calls `toJobExecutionResult(classLoader)` later. Each of calls chain has clear and different semantic, even we add a static method of `ClusterUtils`, either we add method per semantic above or we introduce flag parameter(which is worse). In short, this way is a functional way which combine action with general `thenCompose` combinator. 2. For Exception part. I agree that we should take care of the Exception thrown. I'd like to point out that original `ProgramInvocationException` is actually separated into three cases as above. 1). submission failure. `Dispatcher` already return a JobSubmissionException or its subclasses. 2). execution failure. I think it could be better we add a exception wrapping action in `Dispatcher#requestJobResult` which wraps exception into JobExecutionException 3) deserialize failure. Directly thrown from `toJobExecutionResult(classLoader)`. Besides, these exceptions are different from `ProgramInvocationException`. In my opinion we don't suffer an exception type compatible issue here. So even the exception thrown now are more clear.
---------------------------------------------------------------- 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] With regards, Apache Git Services
