> On Aug. 10, 2015, 7:52 p.m., Vinod Kone wrote: > > src/common/http.hpp, line 73 > > <https://reviews.apache.org/r/37303/diff/1/?file=1036466#file1036466line73> > > > > this helper seems weird. i would rather have isOK()and isAccepted() > > helpers. > > > > anyway, this seems to be used only once. do we even need a helper?
Removed method. > On Aug. 10, 2015, 7:52 p.m., Vinod Kone wrote: > > src/common/http.cpp, line 86 > > <https://reviews.apache.org/r/37303/diff/1/?file=1036467#file1036467line86> > > > > add a default case please, in case someone adds a new supported > > contentType enum. Why ? I want the compilation to fail when a user adds a new supported contentType enum ( -WSwitch ) > On Aug. 10, 2015, 7:52 p.m., Vinod Kone wrote: > > src/common/http.cpp, line 101 > > <https://reviews.apache.org/r/37303/diff/1/?file=1036467#file1036467line101> > > > > ditto. add a default case. Same as above. > On Aug. 10, 2015, 7:52 p.m., Vinod Kone wrote: > > src/scheduler/scheduler.cpp, line 354 > > <https://reviews.apache.org/r/37303/diff/1/?file=1036468#file1036468line354> > > > > you should make sure subscribe got an OK and everything else got > > Accepted. for that this method needs to take Call as argument. Logging an error now for such cases. > On Aug. 10, 2015, 7:52 p.m., Vinod Kone wrote: > > src/scheduler/scheduler.cpp, lines 426-427 > > <https://reviews.apache.org/r/37303/diff/1/?file=1036468#file1036468line426> > > > > When does this happen? This can happen when there might be some corruption of data across the wire, or the server sends a in-complete protobuf/json response. Added the error from decoder to the string too. > On Aug. 10, 2015, 7:52 p.m., Vinod Kone wrote: > > src/scheduler/scheduler.cpp, line 415 > > <https://reviews.apache.org/r/37303/diff/1/?file=1036468#file1036468line415> > > > > When does this happen? Add a comment. > > > > Also, sending a "subscribe with master.." with every error event seems > > a bit redundat? Can we just add documentation to the ERROR event in the > > protobuf saying so? Or are there events where we dont want schedulers to > > oversubscribe? Added. - Anand ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37303/#review94777 ----------------------------------------------------------- On Aug. 11, 2015, 12:17 a.m., Anand Mazumdar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37303/ > ----------------------------------------------------------- > > (Updated Aug. 11, 2015, 12:17 a.m.) > > > Review request for mesos, Ben Mahler and Vinod Kone. > > > Bugs: MESOS-2552 > https://issues.apache.org/jira/browse/MESOS-2552 > > > Repository: mesos > > > Description > ------- > > This changes moves scheduler library to http. The change to remove auth + > install,receive handlers are in other reviews. > > > Diffs > ----- > > src/common/http.hpp 98a12709ae5fcbf4ce3e37d164bff43818096838 > src/common/http.cpp e2ff48ca609f1c97a1b6c20cc6be3d97c3e4bb50 > src/scheduler/scheduler.cpp a8699a7514892a548eb03f9dcb6a1198ce518830 > > Diff: https://reviews.apache.org/r/37303/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Anand Mazumdar > >
