Hi Xiaogang, Thanks for your reply.
According to the feature discussion thread[1] client API enhancement is a planned feature towards 1.10 and thus I think this thread is valid if we can reach a consensus and introduce new client API in this development cycle. Best, tison. [1] https://lists.apache.org/thread.html/22639ca7de62a18f50e90db53e73910bd99b7f00c82f7494f4cb035f@%3Cdev.flink.apache.org%3E SHI Xiaogang <shixiaoga...@gmail.com> 于2019年9月18日周三 下午3:03写道： > Hi Tison, > > Thanks for bringing this. > > I think it's fine to break the back compatibility of client API now that > ClusterClient is not well designed for public usage. > But from my perspective, we should postpone any modification to existing > interfaces until we come to an agreement on new client API. Otherwise, our > users may adapt their implementation more than once. > > Regards, > Xiaogang > > 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 > > >
