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 < > [email protected]> 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 <[email protected]> >> 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 <[email protected]> >>>> wrote: >>>> >>>>> +1 >>>>> Very good idea >>>>> >>>>> >>>>> On Thu, 16 Jul 2015 at 17:57 Fabian Hueske <[email protected]> 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 <[email protected]>: >>>>>> >>>>>>> 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 < >>>>>>> [email protected]> 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 >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> >> >
signature.asc
Description: OpenPGP digital signature
