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
>

Reply via email to