> 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
> 
>

Reply via email to