Hi Jeff, Thanks for your reply.
The ongoing client API enhancement thread[1] is mainly aimed at dealing with issues of our client API, as you mentioned, current client API is no so clean. Because client API naturally becomes public & user-facing inteface it is expected that we start a series of discussions for how the inteface should look like. However, it isn't expected that we have to talk about backward compatibility too much in this scope. I agree that it is painful if we always keep compatibility for non-well-designed API. Even in this specific scenario we bring such to Public. It is mentioned in the discussion under [2] that I think it could be the time or so to discuss our InterfaceAudience policy. At least it would be a pity if we don't address this InterfaceAudience issue towards 2.0. But let's say it could be a separated thread. For expose a thin interface and move all the implementation to AbstractClusterClient, I think the community consensus is towards an ClusterClient interface and thus there is no need for an AbstractClusterClient. For implementation details, it is mainly about a series of #run methods. We will gradually exclude them from ClusterClient and it is the responsibility of Executor in the design document[3] to take care of compilation and deployment. BTW, I take a look at the pull requests you link to. In fact I create a similar issue[4] and also consider simplify code in flink-scala-shell. Let's move the detailed discussion to the corresponding issues and pull requests or start another thread then. I don't intend to cover a lot of concerns generally under this thread :-) Best, tison. [1] https://lists.apache.org/thread.html/ce99cba4a10b9dc40eb729d39910f315ae41d80ec74f09a356c73938@%3Cdev.flink.apache.org%3E [2] https://github.com/apache/flink/commit/dc9e4494dddfed36432e6bbf6cd3231530bc2e01#commitcomment-34980406 [3] https://docs.google.com/document/d/1E-8UjOLz4QPUTxetGWbU23OlsIH9VIdodpTsxwoQTs0/edit# [4] https://issues.apache.org/jira/browse/FLINK-13961 Jeff Zhang <zjf...@gmail.com> 于2019年9月18日周三 上午10:49写道: > Thanks for raising this discussion. Overall +1 to merge NewClusterClient > into ClusterClient. > > 1. I think it is OK to break the backward compatibility. This current > client api is no so clean which already cause issue for downstream project > and flink itself. > In flink scala shell, I notice this kind of non-readable code > Option[Either > [MiniCluster , ClusterClient[_]]]) > > https://github.com/apache/flink/blob/master/flink-scala-shell/src/main/scala/org/apache/flink/api/scala/FlinkShell.scala#L138 > I also created tickets and PR to try to simply it. > https://github.com/apache/flink/pull/8546 > https://github.com/apache/flink/pull/8533 > Personally I don't think we need to keep backward compatibility for > non-well-designed api, otherwise it will bring lots of unnecessary > overhead. > > 2. Another concern is that I notice there're many implementation details in > ClusterClient. I think we should just expose a thin interface, so maybe we > can create interface ClusterClient which includes as less methods as > possible, and move all the implementation to AbstractClusterClient. > > > > > > > > Zili Chen <wander4...@gmail.com> 于2019年9月18日周三 上午9:46写道: > > > Hi devs, > > > > FLINK-14096[1] was created yesterday. It is aimed at merge the bridge > > class NewClusterClient into ClusterClient because with the effort > > under FLINK-10392 this bridge class is no longer necessary. > > > > Technically in current codebase all implementation of interface > > NewClusterClient is subclass of ClusterClient so that the work > > required is no more than move method declaration. It helps we use > > type signature ClusterClient instead of > > <ClusterClient & NewClusterClient or even cannot represent the > > latter if we aren't in a type variable context. This should not affect > > anything internal in Flink scope. > > > > However, as mentioned by Kostas in the JIRA and a previous discussion > > under a commit[2], it seems that we provide some levels of backward > > compatibility for ClusterClient and thus it's better to start a public > > discussion here. > > > > There are two concerns from my side. > > > > 1. How much impact this proposal brings to users programming directly > > to ClusterClient? > > > > The specific changes here are add two methods `submitJob` and > > `requestJobResult` which are already implemented by RestClusterClient > > and MiniClusterClient. Users would only be affected if they create > > a class that inherits ClusterClient and doesn't implement these > > methods. Besides, users who create a class that implements > > NewClusterClient would be affected by the removal of NewClusterClient. > > > > If we have to provide backward compatibility and the impact is no > > further than those above, we can deprecate NewClusterClient, merge > > the methods into ClusterClient with a dummy default like throw > > Exception. > > > > 2. Why do we provide backward compatibility for ClusterClient? > > > > It already surprises Kostas and me while we think ClusterClient is a > > totally internal class which we can evolve regardless of api > > stability. Our community promises api stability by marking class > > and/or method as @Public/@PublicEvolving. It is wried and even > > dangerous we are somehow enforced to provide backward compatibility > > for classes without any annotation. > > > > Besides, as I mention in [2], users who anyway want to program > > directly to internal classes/interfaces are considered to prepare to > > make adaptations when bump version of Flink. > > > > Best, > > tison. > > > > [1] https://issues.apache.org/jira/browse/FLINK-14096 > > [2] > > > > > https://github.com/apache/flink/commit/dc9e4494dddfed36432e6bbf6cd3231530bc2e01 > > > > > -- > Best Regards > > Jeff Zhang >