You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flink.apache.org by "Matthias J. Sax" <mj...@informatik.hu-berlin.de> on 2015/07/16 16:45:16 UTC
[DISCUSS] Unifying client code
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
Re: [DISCUSS] Unifying client code
Posted by "Matthias J. Sax" <mj...@informatik.hu-berlin.de>.
Sorry, but I can't follow. What do you mean by "depends on Client"?
Client.java is updated in PR#642 and PR#750 (adding cancel and stop)
PR#750 introduces STOP after all.
There is also PR#917 and PR#858 both working on session IDs, and both
changing Client.java
So all "depend on Client"...
-Matthias
On 07/17/2015 04:54 PM, Stephan Ewen wrote:
> For any PR that depends on the Client, it would be nice to merge it after
> the client code was updated...
>
> On Fri, Jul 17, 2015 at 4:32 PM, Matthias J. Sax <
> mjsax@informatik.hu-berlin.de> wrote:
>
>> I can take care of open cancel PR. What is the workflow? Fork the
>> branch, apply changes, open new PR?
>>
>> The stop PR is waiting for review already:
>> https://github.com/apache/flink/pull/750
>>
>> Not sure if "cancel + stop" should be merged before or after "session"
>> PR. You should know better which changes are easier to rebase.
>>
>> What about PR https://github.com/apache/flink/pull/904
>> It also applies changes to WebClient. How does it effect (or is effected
>> by) all those changes?
>>
>>
>> I agree, that unifying client code should be the last step.
>>
>>
>> -Matthias
>>
>> On 07/17/2015 04:18 PM, Stephan Ewen wrote:
>>> 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 <mx...@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 <al...@apache.org>
>>>> wrote:
>>>>
>>>>> +1
>>>>> Very good idea
>>>>>
>>>>>
>>>>> On Thu, 16 Jul 2015 at 17:57 Fabian Hueske <fh...@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 <tr...@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
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>>
>
Re: [DISCUSS] Unifying client code
Posted by Stephan Ewen <se...@apache.org>.
For any PR that depends on the Client, it would be nice to merge it after
the client code was updated...
On Fri, Jul 17, 2015 at 4:32 PM, Matthias J. Sax <
mjsax@informatik.hu-berlin.de> wrote:
> I can take care of open cancel PR. What is the workflow? Fork the
> branch, apply changes, open new PR?
>
> The stop PR is waiting for review already:
> https://github.com/apache/flink/pull/750
>
> Not sure if "cancel + stop" should be merged before or after "session"
> PR. You should know better which changes are easier to rebase.
>
> What about PR https://github.com/apache/flink/pull/904
> It also applies changes to WebClient. How does it effect (or is effected
> by) all those changes?
>
>
> I agree, that unifying client code should be the last step.
>
>
> -Matthias
>
> On 07/17/2015 04:18 PM, Stephan Ewen wrote:
> > 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 <mx...@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 <al...@apache.org>
> >> wrote:
> >>
> >>> +1
> >>> Very good idea
> >>>
> >>>
> >>> On Thu, 16 Jul 2015 at 17:57 Fabian Hueske <fh...@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 <tr...@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
> >>>>>>
> >>>>>>
> >>>>>
> >>>>
> >>>
> >>
> >
>
>
Re: [DISCUSS] Unifying client code
Posted by "Matthias J. Sax" <mj...@informatik.hu-berlin.de>.
I can take care of open cancel PR. What is the workflow? Fork the
branch, apply changes, open new PR?
The stop PR is waiting for review already:
https://github.com/apache/flink/pull/750
Not sure if "cancel + stop" should be merged before or after "session"
PR. You should know better which changes are easier to rebase.
What about PR https://github.com/apache/flink/pull/904
It also applies changes to WebClient. How does it effect (or is effected
by) all those changes?
I agree, that unifying client code should be the last step.
-Matthias
On 07/17/2015 04:18 PM, Stephan Ewen wrote:
> 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 <mx...@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 <al...@apache.org>
>> wrote:
>>
>>> +1
>>> Very good idea
>>>
>>>
>>> On Thu, 16 Jul 2015 at 17:57 Fabian Hueske <fh...@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 <tr...@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
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
Re: [DISCUSS] Unifying client code
Posted by Stephan Ewen <se...@apache.org>.
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 <mx...@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 <al...@apache.org>
> wrote:
>
> > +1
> > Very good idea
> >
> >
> > On Thu, 16 Jul 2015 at 17:57 Fabian Hueske <fh...@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 <tr...@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
> > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] Unifying client code
Posted by Maximilian Michels <mx...@apache.org>.
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 <al...@apache.org>
wrote:
> +1
> Very good idea
>
>
> On Thu, 16 Jul 2015 at 17:57 Fabian Hueske <fh...@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 <tr...@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
> > > >
> > > >
> > >
> >
>
Re: [DISCUSS] Unifying client code
Posted by Aljoscha Krettek <al...@apache.org>.
+1
Very good idea
On Thu, 16 Jul 2015 at 17:57 Fabian Hueske <fh...@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 <tr...@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
> > >
> > >
> >
>
Re: [DISCUSS] Unifying client code
Posted by Fabian Hueske <fh...@gmail.com>.
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 <tr...@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
> >
> >
>
Re: [DISCUSS] Unifying client code
Posted by Till Rohrmann <tr...@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
>
>