TisonKun commented on a change in pull request #10311: [FLINK-14762][client]
Enrich JobClient API
URL: https://github.com/apache/flink/pull/10311#discussion_r351183308
##########
File path:
flink-clients/src/main/java/org/apache/flink/client/deployment/AbstractSessionClusterExecutor.java
##########
@@ -50,18 +50,20 @@ public AbstractSessionClusterExecutor(@Nonnull final
ClientFactory clusterClient
}
@Override
- public CompletableFuture<JobClient> execute(@Nonnull final Pipeline
pipeline, @Nonnull final Configuration configuration) throws Exception {
+ public CompletableFuture<? extends JobClient> execute(@Nonnull final
Pipeline pipeline, @Nonnull final Configuration configuration) throws Exception
{
final JobGraph jobGraph = ExecutorUtils.getJobGraph(pipeline,
configuration);
try (final ClusterDescriptor<ClusterID> clusterDescriptor =
clusterClientFactory.createClusterDescriptor(configuration)) {
final ClusterID clusterID =
clusterClientFactory.getClusterId(configuration);
checkState(clusterID != null);
final ClusterClient<ClusterID> clusterClient =
clusterDescriptor.retrieve(clusterID);
- return clusterClient
- .submitJob(jobGraph)
-
.thenApply(JobSubmissionResult::getJobID)
- .thenApply(jobID -> new
ClusterClientJobClientAdapter<>(clusterClient, jobID, false));
+ return
clusterClient.submitJob(jobGraph).thenApply(jobClient -> {
+ if (jobClient instanceof
ClusterClientJobClientAdapter) {
Review comment:
```
I think I get your points here and push a commit
7d98e670ac6e917eb2c121493ba0b5925240899d dedicate for this comment.
Let sync for opinions.
In my understanding the idea is that we should keep initializing closing
actions as near as the job client instanced. I agree that it is a bit tricky
because
(... original thoughts)
```
I have another idea for this stuff because we don't need constructing
JobClient just in-place within `ClusterClient#submitJob` at all(instead, we
return JobID there, and construct JobClient when we actually need one, thus we
are able to rewrite close logic on consturaction). I'm going to a meeting and
possibly back to push a commit reflect my thought hours later.
If we're still unhappy with the implementation I think it would be good we
have a video conversation for consensus later.
----------------------------------------------------------------
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
