flink-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stephan Ewen <se...@apache.org>
Subject Re: [DISCUSS] Unifying client code
Date Fri, 17 Jul 2015 14:18:42 GMT
How about this:

  - I think we should not block on the "cancel" pull request
https://github.com/apache/flink/pull/642
    It is not complex and can be easily forward fitted

  - Let's merge the Client "session" pull request soon.
https://github.com/apache/flink/pull/858
    It changes the assumptions of the client (the client is job independent
and only a gateway to send jobs and trigger actions).

  - After that we can in parallel continue with the "stop" pull request
(not too much logic in the client) and the CLI / Client consolidation.

  - The CLI / Client consolidation should most importantly move the "list"
and "cancel" code to the client.

Makes sense?

For an approximate time line:

The session pull request should be merged soon. IMHO, Max or me should make
a final pass and then sync to add this. I hope it is not more than a few
days.
The pull request is a bit delicate, as the session idea changes the
interaction of client and JobManager a bit, so we'd very much like to get
this really right.

Greetings,
Stephan


On Fri, Jul 17, 2015 at 2:47 PM, Maximilian Michels <mxm@apache.org> wrote:

> I'm also in favor of restructuring the Client. Some of this work has
> already been undergone in this pull request:
> https://github.com/apache/flink/pull/858
>
> It would be great if we could sync such that we do the restructuring once
> the pull request has been merged. We can probably get it in next week.
>
> On Fri, Jul 17, 2015 at 1:52 PM, Aljoscha Krettek <aljoscha@apache.org>
> wrote:
>
> > +1
> > Very good idea
> >
> >
> > On Thu, 16 Jul 2015 at 17:57 Fabian Hueske <fhueske@gmail.com> wrote:
> >
> > > Yes definitely.
> > > The client and submission code is spread out over multiple classes and
> > > different clients follow different paths. This is a bit messy right
> now,
> > > IMO.
> > > A big +1 to unify and restructure the client.
> > >
> > > 2015-07-16 17:52 GMT+02:00 Till Rohrmann <trohrmann@apache.org>:
> > >
> > > > I like the idea to have a single point of access. That would improve
> > > > maintainability and makes the code easier to understand. Thus +1.
> > > >
> > > > On Thu, Jul 16, 2015 at 4:45 PM, Matthias J. Sax <
> > > > mjsax@informatik.hu-berlin.de> wrote:
> > > >
> > > > > Hi,
> > > > >
> > > > > I just had a look into CliFrontend and Client and it seems to me,
> > that
> > > > > there is no uniform design.
> > > > >
> > > > > For example, CliFrontend uses Client to execute "run" and "info"
> > > > > command. However, for "cancel" and "list" it does not (because
> > > > > org.apache.flink.client.program.Client) lacks those methods.
> > > > >
> > > > > I would recommend to extend Client and adapt CliFrontend.
> > > > >
> > > > > There is already a pull request for "cancel":
> > > > > https://github.com/apache/flink/pull/642
> > > > > However, this PR does not adapt CliFrontend and I am not sure if
it
> > > will
> > > > > be finished at all. A three week old discussion resulted in no
> > > progress.
> > > > >
> > > > > In the current pull request for the new STOP signal, there is also
> > some
> > > > > mess with this regard. CliFrontend does not use Client.stop() ->
I
> > will
> > > > > update this soon (this issues was the trigger to discover this
> > "mess"),
> > > > > or include those changes into this work (if we start it).
> > > > >
> > > > > Additionally, we might want to uniform WebClient and job manager
> > > > > WebFrontend, too. I have an open PR that changed WebClient to use
> > > > > CliFrontend to avoid code duplication. But now, this seems not the
> > > right
> > > > > way to go. JobManagerInfoServlet duplicate this code, too.
> > > > >
> > > > > I think Client should be the unique class that offers methods for
> all
> > > > > request and is used by all other clients.
> > > > >
> > > > > What do you think about this?
> > > > >
> > > > >
> > > > > -Matthias
> > > > >
> > > > >
> > > >
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message