> On July 27, 2015, 11:54 p.m., Ben Mahler wrote: > > Nice work Anand! > > > > I left feedback here, but it is all addressed in the diff I sent you over > > email. With the diff applied this patch looks like a shippable step to me. > > Note that per the comments, I also fenced off addFramework for http > > schedulers (much like you've already done here for failoverFramework). In > > both of these, we'll need to set up a readerClosed callback (the equivalent > > of link()). I also noticed that we'll need connection equality for this, so > > I'll get that added for you to work off of (i.e. Pipe::Writer equality > > should be enough). > > > > Can you confirm we have tickets for the following: > > > > * Authenticating the /call endpoint. > > * Extending the existing framework rate limiting functionality to support > > http schedulers.
The Authentication JIRA already exists ( MESOS-2297 ). Would add the rate limiting JIRA. > On July 27, 2015, 11:54 p.m., Ben Mahler wrote: > > src/master/master.cpp, line 1653 > > <https://reviews.apache.org/r/36318/diff/9/?file=1021868#file1021868line1653> > > > > Can this really be a CHECK? > > > > E.g. > > > > HTTP framework F is subscribed. > > A random 'pid'-based KILL is sent with framework id F. > > > > It seems that in this case, we should drop because it's not from the > > subscribed framework, but we'll instead fail this CHECK? > > > > How about: > > > > ``` > > if (framework.pid != from) { > > ... > > } > > ``` > > > > This will handle pid being None. Good catch, fixed all the other occurences. My bad. > On July 27, 2015, 11:54 p.m., Ben Mahler wrote: > > src/master/master.cpp, line 1876 > > <https://reviews.apache.org/r/36318/diff/9/?file=1021868#file1021868line1876> > > > > Any reason you added a TODO in the above send, but not here? Seems we > > also have the `Framework*` here. I can imagine adding a CHECK to send to > > ensure that it's never called with (re-)registration messages when > > http.isSome()? Was this the concern? The TODO was meant for the send(...) above when we did not have a Framework*. My bad. Moved the comment to the other send(...) - Anand ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36318/#review93165 ----------------------------------------------------------- On July 28, 2015, 3:47 a.m., Anand Mazumdar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36318/ > ----------------------------------------------------------- > > (Updated July 28, 2015, 3:47 a.m.) > > > Review request for mesos, Benjamin Hindman, Ben Mahler, Isabel Jimenez, Marco > Massenzio, and Vinod Kone. > > > Bugs: MESOS-2294 > https://issues.apache.org/jira/browse/MESOS-2294 > > > Repository: mesos > > > Description > ------- > > This change refactors the framework struct in master to introduce support for > http frameworks. > - pid becomes a optional field now in the framework struct. > - added optional fields for supporting http frameworks to the framework struct > > > Diffs > ----- > > src/common/http.hpp 765860fa7d0ce354320e9d293d09719be87efca0 > src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 > src/master/master.hpp 2c924addfb4c52d3048ee6ded13ce638145cc93f > src/master/master.cpp a8a195df07b5a97fdba7dfc5f312bbfa85a0d510 > > Diff: https://reviews.apache.org/r/36318/diff/ > > > Testing > ------- > > make check + tests now go in a separate patch now. > > > Thanks, > > Anand Mazumdar > >
