Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-08-10 Thread Anand Mazumdar

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/
---

(Updated Aug. 10, 2015, 7:20 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Re-opened to remove stale depedency.


Bugs: MESOS-2294
https://issues.apache.org/jira/browse/MESOS-2294


Repository: mesos


Description
---

Split review out of r36318. This change adds the functionality of making a http 
call for subscribe and the master responding with a subscribed event on the 
persistent stream.

Also added functionality for framework failover equivalent of re-register. It 
should now be possible to merge the subscribed(...) introduced in this review 
and the re-factor that happened in MESOS-3182.

- Made a new function for exited()/failoverFramework for http frameworks that 
invoke into the common continuation function for pid/http frameworks thereafter.
- The re-register functionality equivalent goes in _subscribe(...)


Diffs
-

  src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
  src/master/master.hpp 53420ca7d503296fbe11b1ea0795afe2ebf86255 
  src/master/master.cpp d699e4bc3cf734a516a6baf329919e04744b5702 

Diff: https://reviews.apache.org/r/36720/diff/


Testing
---

make check + adding tests in a different patch.


Thanks,

Anand Mazumdar



Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-08-07 Thread Ben Mahler

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/#review94496
---

Ship it!


I left some comments here so you could see the issues I noticed as I went over 
this, mostly around upgrading and downgrading across pid and http.

I've applied fixes for all of these to avoid more round-trips and will get this 
committed shortly, nice work!


src/master/master.cpp (line 4950)
https://reviews.apache.org/r/36720/#comment149133

Much like when we remove a pid-based framework, we need to wipe the 
authentication related data here.



src/master/master.cpp (lines 4963 - 4968)
https://reviews.apache.org/r/36720/#comment149135

Perhaps we should just push the unsetting of the other connection into 
updateConnection and have one for both http and pid.



src/master/master.cpp (line 4967)
https://reviews.apache.org/r/36720/#comment149136

We need to remove the old one from `authenticated` and `principals` here 
when updrading to http, much like we do when a pid based framework is removed.

Yes.. this stuff is a nasty mess, we need to make failover as simple as the 
composition of: disconnect - reconnect!



src/master/master.cpp (line 4983)
https://reviews.apache.org/r/36720/#comment149129

This check will crash when an http scheduler is downgrading to a pid 
framework.



src/master/master.cpp (lines 5010 - 5015)
https://reviews.apache.org/r/36720/#comment149132

This won't work for downgrades from http to pid, since there was no 
principal in the map yet.



src/master/master.cpp (lines 5039 - 5040)
https://reviews.apache.org/r/36720/#comment149138

Could we do this before reactivating? Note that your comment seems to hint 
that it must come after for some reason, but it doesn't need to.


- Ben Mahler


On Aug. 7, 2015, 2:27 a.m., Anand Mazumdar wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36720/
 ---
 
 (Updated Aug. 7, 2015, 2:27 a.m.)
 
 
 Review request for mesos, Ben Mahler and Vinod Kone.
 
 
 Bugs: MESOS-2294
 https://issues.apache.org/jira/browse/MESOS-2294
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Split review out of r36318. This change adds the functionality of making a 
 http call for subscribe and the master responding with a subscribed event on 
 the persistent stream.
 
 Also added functionality for framework failover equivalent of re-register. It 
 should now be possible to merge the subscribed(...) introduced in this review 
 and the re-factor that happened in MESOS-3182.
 
 - Made a new function for exited()/failoverFramework for http frameworks that 
 invoke into the common continuation function for pid/http frameworks 
 thereafter.
 - The re-register functionality equivalent goes in _subscribe(...)
 
 
 Diffs
 -
 
   src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
   src/master/master.hpp 53420ca7d503296fbe11b1ea0795afe2ebf86255 
   src/master/master.cpp d699e4bc3cf734a516a6baf329919e04744b5702 
 
 Diff: https://reviews.apache.org/r/36720/diff/
 
 
 Testing
 ---
 
 make check + adding tests in a different patch.
 
 
 Thanks,
 
 Anand Mazumdar
 




Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-08-06 Thread Anand Mazumdar


 On Aug. 6, 2015, 6:39 p.m., Ben Mahler wrote:
  src/master/master.cpp, line 4990
  https://reviews.apache.org/r/36720/diff/9/?file=1033626#file1033626line4990
 
  Don't we still want this since we call pid.get()?

Persisted the CHECK_SOME. Removed the misleading message though as we have now 
implemented http framework failover.


- Anand


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/#review94428
---


On Aug. 6, 2015, 4:25 p.m., Anand Mazumdar wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36720/
 ---
 
 (Updated Aug. 6, 2015, 4:25 p.m.)
 
 
 Review request for mesos, Ben Mahler and Vinod Kone.
 
 
 Bugs: MESOS-2294
 https://issues.apache.org/jira/browse/MESOS-2294
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Split review out of r36318. This change adds the functionality of making a 
 http call for subscribe and the master responding with a subscribed event on 
 the persistent stream.
 
 Also added functionality for framework failover equivalent of re-register. It 
 should now be possible to merge the subscribed(...) introduced in this review 
 and the re-factor that happened in MESOS-3182.
 
 - Made a new function for exited()/failoverFramework for http frameworks that 
 invoke into the common continuation function for pid/http frameworks 
 thereafter.
 - The re-register functionality equivalent goes in _subscribe(...)
 
 
 Diffs
 -
 
   src/common/http.hpp 9e4290f9e1f72730f63466d498a953cc5031a92b 
   src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 
   src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
   src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
   src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 
 
 Diff: https://reviews.apache.org/r/36720/diff/
 
 
 Testing
 ---
 
 make check + adding tests in a different patch.
 
 
 Thanks,
 
 Anand Mazumdar
 




Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-08-06 Thread Anand Mazumdar

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/
---

(Updated Aug. 7, 2015, 2:27 a.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Modified signature for subscribe(...), added _failoverFramework. Modified some 
TODO messages to be more explicit.


Bugs: MESOS-2294
https://issues.apache.org/jira/browse/MESOS-2294


Repository: mesos


Description
---

Split review out of r36318. This change adds the functionality of making a http 
call for subscribe and the master responding with a subscribed event on the 
persistent stream.

Also added functionality for framework failover equivalent of re-register. It 
should now be possible to merge the subscribed(...) introduced in this review 
and the re-factor that happened in MESOS-3182.

- Made a new function for exited()/failoverFramework for http frameworks that 
invoke into the common continuation function for pid/http frameworks thereafter.
- The re-register functionality equivalent goes in _subscribe(...)


Diffs (updated)
-

  src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
  src/master/master.hpp 53420ca7d503296fbe11b1ea0795afe2ebf86255 
  src/master/master.cpp d699e4bc3cf734a516a6baf329919e04744b5702 

Diff: https://reviews.apache.org/r/36720/diff/


Testing
---

make check + adding tests in a different patch.


Thanks,

Anand Mazumdar



Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-08-06 Thread Ben Mahler

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/#review94428
---


Thanks, I'll get the HttpConnection change committed, left some notes about 
things I updated.

Also opened some issues in the subscribe path that I noticed while doing this.


src/common/http.hpp (line 50)
https://reviews.apache.org/r/36720/#comment149022

Hm.. let's at least say that it's based on the content type.



src/common/http.cpp (line 49)
https://reviews.apache.org/r/36720/#comment149021

No need for std:: here



src/common/http.cpp (lines 61 - 63)
https://reviews.apache.org/r/36720/#comment149020

How about we just put UNREACHABLE at the bottom outside the switch? That 
way, we don't lose the switch warning for missing enum cases, right?



src/master/master.hpp (lines 1542 - 1544)
https://reviews.apache.org/r/36720/#comment149023

Let's have the master decide when to set up this callback. On that note, 
how about we add a `closed()` on the HttpConnection as well?



src/master/master.cpp (line 2216)
https://reviews.apache.org/r/36720/#comment149024

This change looks like a regression, now we're sending to 
'`framework-pid`' instead of '`from`'?



src/master/master.cpp 
https://reviews.apache.org/r/36720/#comment149027

Don't we still want this since we call pid.get()?


- Ben Mahler


On Aug. 6, 2015, 4:25 p.m., Anand Mazumdar wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36720/
 ---
 
 (Updated Aug. 6, 2015, 4:25 p.m.)
 
 
 Review request for mesos, Ben Mahler and Vinod Kone.
 
 
 Bugs: MESOS-2294
 https://issues.apache.org/jira/browse/MESOS-2294
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Split review out of r36318. This change adds the functionality of making a 
 http call for subscribe and the master responding with a subscribed event on 
 the persistent stream.
 
 Also added functionality for framework failover equivalent of re-register. It 
 should now be possible to merge the subscribed(...) introduced in this review 
 and the re-factor that happened in MESOS-3182.
 
 - Made a new function for exited()/failoverFramework for http frameworks that 
 invoke into the common continuation function for pid/http frameworks 
 thereafter.
 - The re-register functionality equivalent goes in _subscribe(...)
 
 
 Diffs
 -
 
   src/common/http.hpp 9e4290f9e1f72730f63466d498a953cc5031a92b 
   src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 
   src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
   src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
   src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 
 
 Diff: https://reviews.apache.org/r/36720/diff/
 
 
 Testing
 ---
 
 make check + adding tests in a different patch.
 
 
 Thanks,
 
 Anand Mazumdar
 




Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-08-06 Thread Vinod Kone

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/#review94413
---



src/common/http.cpp (line 60)
https://reviews.apache.org/r/36720/#comment148999

need a return here to aviod compiler warning.


- Vinod Kone


On Aug. 6, 2015, 4:08 a.m., Anand Mazumdar wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36720/
 ---
 
 (Updated Aug. 6, 2015, 4:08 a.m.)
 
 
 Review request for mesos, Ben Mahler and Vinod Kone.
 
 
 Bugs: MESOS-2294
 https://issues.apache.org/jira/browse/MESOS-2294
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Split review out of r36318. This change adds the functionality of making a 
 http call for subscribe and the master responding with a subscribed event on 
 the persistent stream.
 
 Also added functionality for framework failover equivalent of re-register. It 
 should now be possible to merge the subscribed(...) introduced in this review 
 and the re-factor that happened in MESOS-3182.
 
 - Made a new function for exited()/failoverFramework for http frameworks that 
 invoke into the common continuation function for pid/http frameworks 
 thereafter.
 - The re-register functionality equivalent goes in _subscribe(...)
 
 
 Diffs
 -
 
   src/common/http.hpp 9e4290f9e1f72730f63466d498a953cc5031a92b 
   src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 
   src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
   src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
   src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 
 
 Diff: https://reviews.apache.org/r/36720/diff/
 
 
 Testing
 ---
 
 make check + adding tests in a different patch.
 
 
 Thanks,
 
 Anand Mazumdar
 




Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-08-06 Thread Anand Mazumdar


 On Aug. 6, 2015, 4:10 p.m., Vinod Kone wrote:
  src/common/http.cpp, line 60
  https://reviews.apache.org/r/36720/diff/8/?file=1033227#file1033227line60
 
  need a return here to aviod compiler warning.

I am putting in a fix for that. This looks to be as a GCC bug. With 
-Werror=uninitialized, -Wswitch turned on in our code. It shouldn't ever 
complain. Clang rightfully figures this out but GCC does not. Would go ahead 
and put in a default: UNREACHABLE() workaround here for now.


- Anand


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/#review94413
---


On Aug. 6, 2015, 4:08 a.m., Anand Mazumdar wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36720/
 ---
 
 (Updated Aug. 6, 2015, 4:08 a.m.)
 
 
 Review request for mesos, Ben Mahler and Vinod Kone.
 
 
 Bugs: MESOS-2294
 https://issues.apache.org/jira/browse/MESOS-2294
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Split review out of r36318. This change adds the functionality of making a 
 http call for subscribe and the master responding with a subscribed event on 
 the persistent stream.
 
 Also added functionality for framework failover equivalent of re-register. It 
 should now be possible to merge the subscribed(...) introduced in this review 
 and the re-factor that happened in MESOS-3182.
 
 - Made a new function for exited()/failoverFramework for http frameworks that 
 invoke into the common continuation function for pid/http frameworks 
 thereafter.
 - The re-register functionality equivalent goes in _subscribe(...)
 
 
 Diffs
 -
 
   src/common/http.hpp 9e4290f9e1f72730f63466d498a953cc5031a92b 
   src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 
   src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
   src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
   src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 
 
 Diff: https://reviews.apache.org/r/36720/diff/
 
 
 Testing
 ---
 
 make check + adding tests in a different patch.
 
 
 Thanks,
 
 Anand Mazumdar
 




Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-08-06 Thread Anand Mazumdar

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/
---

(Updated Aug. 6, 2015, 4:25 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Added a UNREACHABLE for default case to put off GCC for now.


Bugs: MESOS-2294
https://issues.apache.org/jira/browse/MESOS-2294


Repository: mesos


Description
---

Split review out of r36318. This change adds the functionality of making a http 
call for subscribe and the master responding with a subscribed event on the 
persistent stream.

Also added functionality for framework failover equivalent of re-register. It 
should now be possible to merge the subscribed(...) introduced in this review 
and the re-factor that happened in MESOS-3182.

- Made a new function for exited()/failoverFramework for http frameworks that 
invoke into the common continuation function for pid/http frameworks thereafter.
- The re-register functionality equivalent goes in _subscribe(...)


Diffs (updated)
-

  src/common/http.hpp 9e4290f9e1f72730f63466d498a953cc5031a92b 
  src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 
  src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
  src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
  src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 

Diff: https://reviews.apache.org/r/36720/diff/


Testing
---

make check + adding tests in a different patch.


Thanks,

Anand Mazumdar



Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-08-05 Thread Anand Mazumdar

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/
---

(Updated Aug. 6, 2015, 4:08 a.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Added a continuation function for failoverFramework(...) that is common to both 
http and pid frameworks. Made the readerClosed callback as part of addFramework 
similar to link(...)


Bugs: MESOS-2294
https://issues.apache.org/jira/browse/MESOS-2294


Repository: mesos


Description
---

Split review out of r36318. This change adds the functionality of making a http 
call for subscribe and the master responding with a subscribed event on the 
persistent stream.

Also added functionality for framework failover equivalent of re-register. It 
should now be possible to merge the subscribed(...) introduced in this review 
and the re-factor that happened in MESOS-3182.

- Made a new function for exited()/failoverFramework for http frameworks that 
invoke into the common continuation function for pid/http frameworks thereafter.
- The re-register functionality equivalent goes in _subscribe(...)


Diffs (updated)
-

  src/common/http.hpp 9e4290f9e1f72730f63466d498a953cc5031a92b 
  src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 
  src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
  src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
  src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 

Diff: https://reviews.apache.org/r/36720/diff/


Testing
---

make check + adding tests in a different patch.


Thanks,

Anand Mazumdar



Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-08-05 Thread Anand Mazumdar

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/
---

(Updated Aug. 5, 2015, 10:37 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Added a new HttpConnection struct instead of optimistically creating framework 
objects and deleting them for re-registration.


Bugs: MESOS-2294
https://issues.apache.org/jira/browse/MESOS-2294


Repository: mesos


Description
---

Split review out of r36318. This change adds the functionality of making a http 
call for subscribe and the master responding with a subscribed event on the 
persistent stream.

Also added functionality for framework failover equivalent of re-register. It 
should now be possible to merge the subscribed(...) introduced in this review 
and the re-factor that happened in MESOS-3182.

- Made a new function for exited()/failoverFramework for http frameworks that 
invoke into the common continuation function for pid/http frameworks thereafter.
- The re-register functionality equivalent goes in _subscribe(...)


Diffs (updated)
-

  src/common/http.hpp 9e4290f9e1f72730f63466d498a953cc5031a92b 
  src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 
  src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
  src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
  src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 

Diff: https://reviews.apache.org/r/36720/diff/


Testing
---

make check + adding tests in a different patch.


Thanks,

Anand Mazumdar



Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-08-04 Thread Anand Mazumdar

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/
---

(Updated Aug. 4, 2015, 10 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Rebased from master from Ben's latest commit.


Bugs: MESOS-2294
https://issues.apache.org/jira/browse/MESOS-2294


Repository: mesos


Description
---

Split review out of r36318. This change adds the functionality of making a http 
call for subscribe and the master responding with a subscribed event on the 
persistent stream.

Also added functionality for framework failover equivalent of re-register. It 
should now be possible to merge the subscribed(...) introduced in this review 
and the re-factor that happened in MESOS-3182.

- Made a new function for exited()/failoverFramework for http frameworks that 
invoke into the common continuation function for pid/http frameworks thereafter.
- The re-register functionality equivalent goes in _subscribe(...)


Diffs (updated)
-

  src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
  src/master/master.hpp cd0a5c863d4f74c3bcfd61d8e3046b3ab249be67 
  src/master/master.cpp 87e11d512f68e2b7285ee1901d422991208baf15 

Diff: https://reviews.apache.org/r/36720/diff/


Testing
---

make check + adding tests in a different patch.


Thanks,

Anand Mazumdar



Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-08-04 Thread Anand Mazumdar

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/
---

(Updated Aug. 4, 2015, 11:57 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Rebased from master after Ben's commit for exited


Bugs: MESOS-2294
https://issues.apache.org/jira/browse/MESOS-2294


Repository: mesos


Description
---

Split review out of r36318. This change adds the functionality of making a http 
call for subscribe and the master responding with a subscribed event on the 
persistent stream.

Also added functionality for framework failover equivalent of re-register. It 
should now be possible to merge the subscribed(...) introduced in this review 
and the re-factor that happened in MESOS-3182.

- Made a new function for exited()/failoverFramework for http frameworks that 
invoke into the common continuation function for pid/http frameworks thereafter.
- The re-register functionality equivalent goes in _subscribe(...)


Diffs (updated)
-

  src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
  src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
  src/master/master.cpp 5aa0a5410804fe16abd50b6953f1ffe46a019ecf 

Diff: https://reviews.apache.org/r/36720/diff/


Testing
---

make check + adding tests in a different patch.


Thanks,

Anand Mazumdar



Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-08-04 Thread Ben Mahler


 On Aug. 4, 2015, 9:07 p.m., Ben Mahler wrote:
  I'll get the /call validation committed.

Let's follow up to get tests added for each of the error cases in /call.


- Ben


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/#review94111
---


On Aug. 4, 2015, 10 p.m., Anand Mazumdar wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36720/
 ---
 
 (Updated Aug. 4, 2015, 10 p.m.)
 
 
 Review request for mesos, Ben Mahler and Vinod Kone.
 
 
 Bugs: MESOS-2294
 https://issues.apache.org/jira/browse/MESOS-2294
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Split review out of r36318. This change adds the functionality of making a 
 http call for subscribe and the master responding with a subscribed event on 
 the persistent stream.
 
 Also added functionality for framework failover equivalent of re-register. It 
 should now be possible to merge the subscribed(...) introduced in this review 
 and the re-factor that happened in MESOS-3182.
 
 - Made a new function for exited()/failoverFramework for http frameworks that 
 invoke into the common continuation function for pid/http frameworks 
 thereafter.
 - The re-register functionality equivalent goes in _subscribe(...)
 
 
 Diffs
 -
 
   src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
   src/master/master.hpp cd0a5c863d4f74c3bcfd61d8e3046b3ab249be67 
   src/master/master.cpp 87e11d512f68e2b7285ee1901d422991208baf15 
 
 Diff: https://reviews.apache.org/r/36720/diff/
 
 
 Testing
 ---
 
 make check + adding tests in a different patch.
 
 
 Thanks,
 
 Anand Mazumdar
 




Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-08-04 Thread Ben Mahler

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/#review94130
---


I'll get the exited changes committed.


src/master/master.cpp (line 969)
https://reviews.apache.org/r/36720/#comment148645

Hm.. let's check that the framework ids are equal when the writers match.



src/master/master.cpp (line 971)
https://reviews.apache.org/r/36720/#comment148644

s/exited/disconnection/



src/master/master.cpp (line 1021)
https://reviews.apache.org/r/36720/#comment148648

Let's put `_exited` after both `exited` implementations, since it is a 
shared continuation.


- Ben Mahler


On Aug. 4, 2015, 10 p.m., Anand Mazumdar wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36720/
 ---
 
 (Updated Aug. 4, 2015, 10 p.m.)
 
 
 Review request for mesos, Ben Mahler and Vinod Kone.
 
 
 Bugs: MESOS-2294
 https://issues.apache.org/jira/browse/MESOS-2294
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Split review out of r36318. This change adds the functionality of making a 
 http call for subscribe and the master responding with a subscribed event on 
 the persistent stream.
 
 Also added functionality for framework failover equivalent of re-register. It 
 should now be possible to merge the subscribed(...) introduced in this review 
 and the re-factor that happened in MESOS-3182.
 
 - Made a new function for exited()/failoverFramework for http frameworks that 
 invoke into the common continuation function for pid/http frameworks 
 thereafter.
 - The re-register functionality equivalent goes in _subscribe(...)
 
 
 Diffs
 -
 
   src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
   src/master/master.hpp cd0a5c863d4f74c3bcfd61d8e3046b3ab249be67 
   src/master/master.cpp 87e11d512f68e2b7285ee1901d422991208baf15 
 
 Diff: https://reviews.apache.org/r/36720/diff/
 
 
 Testing
 ---
 
 make check + adding tests in a different patch.
 
 
 Thanks,
 
 Anand Mazumdar
 




Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-08-04 Thread Anand Mazumdar


 On July 29, 2015, 7:12 p.m., Ben Mahler wrote:
  src/master/master.cpp, lines 2128-2159
  https://reviews.apache.org/r/36720/diff/3/?file=1021871#file1021871line2128
 
  Any reason you're skipping validation (authorization) of the framework? 
  Looks like we should pull out the pid specific code from validate for now: 
  it looks like we already have to do synchronous checks inside the 
  continuations anyway. For example, `_reregisterFramework` checks to see if 
  it is authenticated, but it is missing the check that it is the right 
  principal!
  
  Looks like we should s/validate/authorize/ and pull out an 
  `isAuthenticated` that we can call synchronously as well.
 
 Anand Mazumdar wrote:
 Primarily due to the reasons you had mentioned earlier. There was a lot 
 of pid specific code in validate. Also , call validation in general were 
 supposed to be handled in MESOS-2497, so I refrained from making the changes. 
 Looks like you already did them in r36919, Thanks !

Would take up validation in a different patch as we had discussed earlier. I am 
dropping the issue for now. Let me know if you think otherwise.


- Anand


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/#review93499
---


On Aug. 4, 2015, 4:33 p.m., Anand Mazumdar wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36720/
 ---
 
 (Updated Aug. 4, 2015, 4:33 p.m.)
 
 
 Review request for mesos, Ben Mahler and Vinod Kone.
 
 
 Bugs: MESOS-2294
 https://issues.apache.org/jira/browse/MESOS-2294
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Split review out of r36318. This change adds the functionality of making a 
 http call for subscribe and the master responding with a subscribed event on 
 the persistent stream.
 
 Also added functionality for framework failover equivalent of re-register. It 
 should now be possible to merge the subscribed(...) introduced in this review 
 and the re-factor that happened in MESOS-3182.
 
 - Made a new function for exited()/failoverFramework for http frameworks that 
 invoke into the common continuation function for pid/http frameworks 
 thereafter.
 - The re-register functionality equivalent goes in _subscribe(...)
 
 
 Diffs
 -
 
   src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f 
   src/master/master.hpp ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc 
   src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 
 
 Diff: https://reviews.apache.org/r/36720/diff/
 
 
 Testing
 ---
 
 make check + adding tests in a different patch.
 
 
 Thanks,
 
 Anand Mazumdar
 




Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-08-04 Thread Anand Mazumdar

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/
---

(Updated Aug. 4, 2015, 4:33 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Added functionality to failover http frameworks.


Bugs: MESOS-2294
https://issues.apache.org/jira/browse/MESOS-2294


Repository: mesos


Description (updated)
---

Split review out of r36318. This change adds the functionality of making a http 
call for subscribe and the master responding with a subscribed event on the 
persistent stream.

Also added functionality for framework failover equivalent of re-register. It 
should now be possible to merge the subscribed(...) introduced in this review 
and the re-factor that happened in MESOS-3182.

- Made a new function for exited()/failoverFramework for http frameworks that 
invoke into the common continuation function for pid/http frameworks thereafter.
- The re-register functionality equivalent goes in _subscribe(...)


Diffs (updated)
-

  src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f 
  src/master/master.hpp ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc 
  src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 

Diff: https://reviews.apache.org/r/36720/diff/


Testing (updated)
---

make check + adding tests in a different patch.


Thanks,

Anand Mazumdar



Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-08-04 Thread Ben Mahler

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/#review94111
---


I'll get the /call validation committed.


src/master/http.cpp (line 355)
https://reviews.apache.org/r/36720/#comment148622

Need an else here for UnsupportedMediaType, I'll add this in.


- Ben Mahler


On Aug. 4, 2015, 4:33 p.m., Anand Mazumdar wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36720/
 ---
 
 (Updated Aug. 4, 2015, 4:33 p.m.)
 
 
 Review request for mesos, Ben Mahler and Vinod Kone.
 
 
 Bugs: MESOS-2294
 https://issues.apache.org/jira/browse/MESOS-2294
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Split review out of r36318. This change adds the functionality of making a 
 http call for subscribe and the master responding with a subscribed event on 
 the persistent stream.
 
 Also added functionality for framework failover equivalent of re-register. It 
 should now be possible to merge the subscribed(...) introduced in this review 
 and the re-factor that happened in MESOS-3182.
 
 - Made a new function for exited()/failoverFramework for http frameworks that 
 invoke into the common continuation function for pid/http frameworks 
 thereafter.
 - The re-register functionality equivalent goes in _subscribe(...)
 
 
 Diffs
 -
 
   src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f 
   src/master/master.hpp ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc 
   src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 
 
 Diff: https://reviews.apache.org/r/36720/diff/
 
 
 Testing
 ---
 
 make check + adding tests in a different patch.
 
 
 Thanks,
 
 Anand Mazumdar
 




Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-07-29 Thread Anand Mazumdar


 On July 29, 2015, 7:12 p.m., Ben Mahler wrote:
  src/master/master.cpp, lines 2128-2159
  https://reviews.apache.org/r/36720/diff/3/?file=1021871#file1021871line2128
 
  Any reason you're skipping validation (authorization) of the framework? 
  Looks like we should pull out the pid specific code from validate for now: 
  it looks like we already have to do synchronous checks inside the 
  continuations anyway. For example, `_reregisterFramework` checks to see if 
  it is authenticated, but it is missing the check that it is the right 
  principal!
  
  Looks like we should s/validate/authorize/ and pull out an 
  `isAuthenticated` that we can call synchronously as well.

Primarily due to the reasons you had mentioned earlier. There was a lot of pid 
specific code in validate. Also , call validation in general were supposed to 
be handled in MESOS-2497, so I refrained from making the changes. Looks like 
you already did them in r36919, Thanks !


 On July 29, 2015, 7:12 p.m., Ben Mahler wrote:
  src/master/master.cpp, lines 2162-2193
  https://reviews.apache.org/r/36720/diff/3/?file=1021871#file1021871line2162
 
  This was a bit confusing, since it says registering but you're 
  planning to also have it include re-registration? This looks like 
  `_subscribe` given it is the continuation.

Yes, I intend to make re-register also call into this function. You are also 
right about this being the _subscribe.

Since I had not yet taken care of validation as per my earlier argument, I went 
ahead with naming it as subscribe for now. When we have the validation routine 
, it would just become a continuation function and can be renamed. Also , 
failover logic would be implemented inside this function and the existing 
_reregisterFramework(...)

would be calling into this function. What do you think ?


 On July 29, 2015, 7:12 p.m., Ben Mahler wrote:
  src/master/http.cpp, lines 324-379
  https://reviews.apache.org/r/36720/diff/3/?file=1021869#file1021869line324
 
  JSON should be do-able already, here's a suggestion to clean this up a 
  bit (note that it is handling JSON):
  
  ```
  FutureResponse Master::Http::call(const Request request) const
  {
scheduler::Call call;
  
// TODO(anand): Content type values are case-insensitive.
Optionstring contentType = request.headers.get(Content-Type);
  
if (contentType.isNone()) {
  return BadRequest(Expecting 'Content-Type' to be present);
}
  
if (contentType.get() == APPLICATION_PROTOBUF) {
  if (!call.ParseFromString(request.body)) {
return BadRequest(Failed to deserialize body into Call 
  protobuf);
  }
} else if (contentType.get() == APPLICATION_JSON) {
  TryJSON::Value value = JSON::parse(request.body);
  
  if (value.isError()) {
return BadRequest(Failed to parse body into JSON:  + 
  value.error());
  }
  
  Tryscheduler::Call parse =
::protobuf::parsescheduler::Call(value.get());
  
  if (parse.isError()) {
return BadRequest(Failed to convert JSON into Call protobuf:  +
  parse.error());
  }
  
  call = parse.get();
}
  
// Default to sending back JSON.
ContentType responseContentType = ContentType::JSON;
  
// TODO(anand): Use request.accepts() once available.
Optionstring acceptType = request.headers.get(Accept);
  
if (acceptType.get() == APPLICATION_PROTOBUF) {
  responseContentType = ContentType::PROTOBUF;
}
  
switch (call.type()) {
  case scheduler::Call::SUBSCRIBE: {
Pipe pipe;
OK ok;
  
ok.type = Response::PIPE;
ok.reader = pipe.reader();
  
master-subscribeHttp(
call.subscribe(),
responseContentType,
pipe.writer());
  
return ok;
  }
  default:
break;
}
  
return NotImplemented();
  }
  ```

Thanks, I would include the JSON logic. I thought adding the JSON workflow 
might be more convoluted then this and hence wanted it to be part of a 
different patch, my bad.


- Anand


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/#review93499
---


On July 25, 2015, 2:32 p.m., Anand Mazumdar wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36720/
 ---
 
 (Updated July 25, 2015, 2:32 p.m.)
 
 
 Review request for mesos and Ben Mahler.
 
 
 Bugs: MESOS-2294
 

Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-07-29 Thread Ben Mahler

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/#review93499
---


Looks like this needs a rebase? Wasn't able to get it applied without `--3way`.

I'm going to pull out the call Call validation to hopefully make this a bit 
easier. Per my comment below, I'd suggest we try to tackle the 'validate' 
refactor in an independent patch to pull apart the authorization vs 
authentication checks.


src/master/http.cpp (lines 324 - 379)
https://reviews.apache.org/r/36720/#comment147888

JSON should be do-able already, here's a suggestion to clean this up a bit 
(note that it is handling JSON):

```
FutureResponse Master::Http::call(const Request request) const
{
  scheduler::Call call;

  // TODO(anand): Content type values are case-insensitive.
  Optionstring contentType = request.headers.get(Content-Type);

  if (contentType.isNone()) {
return BadRequest(Expecting 'Content-Type' to be present);
  }

  if (contentType.get() == APPLICATION_PROTOBUF) {
if (!call.ParseFromString(request.body)) {
  return BadRequest(Failed to deserialize body into Call protobuf);
}
  } else if (contentType.get() == APPLICATION_JSON) {
TryJSON::Value value = JSON::parse(request.body);

if (value.isError()) {
  return BadRequest(Failed to parse body into JSON:  + value.error());
}

Tryscheduler::Call parse =
  ::protobuf::parsescheduler::Call(value.get());

if (parse.isError()) {
  return BadRequest(Failed to convert JSON into Call protobuf:  +
parse.error());
}

call = parse.get();
  }

  // Default to sending back JSON.
  ContentType responseContentType = ContentType::JSON;

  // TODO(anand): Use request.accepts() once available.
  Optionstring acceptType = request.headers.get(Accept);

  if (acceptType.get() == APPLICATION_PROTOBUF) {
responseContentType = ContentType::PROTOBUF;
  }

  switch (call.type()) {
case scheduler::Call::SUBSCRIBE: {
  Pipe pipe;
  OK ok;

  ok.type = Response::PIPE;
  ok.reader = pipe.reader();

  master-subscribeHttp(
  call.subscribe(),
  responseContentType,
  pipe.writer());

  return ok;
}
default:
  break;
  }

  return NotImplemented();
}
```



src/master/master.cpp (lines 2110 - 2141)
https://reviews.apache.org/r/36720/#comment147890

Any reason you're skipping validation (authorization) of the framework? 
Looks like we should pull out the pid specific code from validate for now: it 
looks like we already have to do synchronous checks inside the continuations 
anyway. For example, `_reregisterFramework` checks to see if it is 
authenticated, but it is missing the check that it is the right principal!

Looks like we should s/validate/authorize/ and pull out an 
`isAuthenticated` that we can call synchronously as well.



src/master/master.cpp (lines 2144 - 2175)
https://reviews.apache.org/r/36720/#comment147889

This was a bit confusing, since it says registering but you're planning 
to also have it include re-registration? This looks like `_subscribe` given it 
is the continuation.


- Ben Mahler


On July 25, 2015, 2:32 p.m., Anand Mazumdar wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36720/
 ---
 
 (Updated July 25, 2015, 2:32 p.m.)
 
 
 Review request for mesos and Ben Mahler.
 
 
 Bugs: MESOS-2294
 https://issues.apache.org/jira/browse/MESOS-2294
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Split review out of r36318. This change adds the functionality of making a 
 http call for subscribe and the master responding with a subscribed event on 
 the persistent stream.
 
 
 Diffs
 -
 
   src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 
   src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 
   src/master/master.cpp 613a011e205611702921179b6c436d62447e2dca 
   src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac 
 
 Diff: https://reviews.apache.org/r/36720/diff/
 
 
 Testing
 ---
 
 make check + test.
 
 
 Thanks,
 
 Anand Mazumdar
 




Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-07-27 Thread Anand Mazumdar


 On July 26, 2015, 9:37 p.m., Jojy Varghese wrote:
  src/master/http.cpp, line 339
  https://reviews.apache.org/r/36720/diff/3/?file=1021869#file1021869line339
 
  complexity of a function is measured by the nested if condition in it. 
  Maybe we accomplish the same using simple small functions ?

Yes, we intend to move these in the near future. To add a bit of context here, 
the validation logic would be handled in MESOS-2497. I added the band-aid logic 
here to prevent the master from crashing for now.

Here is the TODO mentioned in the code: // Move this to unmarshal(...) method.


- Anand


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/#review93060
---


On July 25, 2015, 2:32 p.m., Anand Mazumdar wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36720/
 ---
 
 (Updated July 25, 2015, 2:32 p.m.)
 
 
 Review request for mesos and Ben Mahler.
 
 
 Bugs: MESOS-2294
 https://issues.apache.org/jira/browse/MESOS-2294
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Split review out of r36318. This change adds the functionality of making a 
 http call for subscribe and the master responding with a subscribed event on 
 the persistent stream.
 
 
 Diffs
 -
 
   src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 
   src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 
   src/master/master.cpp 613a011e205611702921179b6c436d62447e2dca 
   src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac 
 
 Diff: https://reviews.apache.org/r/36720/diff/
 
 
 Testing
 ---
 
 make check + test.
 
 
 Thanks,
 
 Anand Mazumdar
 




Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-07-26 Thread Jojy Varghese

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/#review93060
---



src/master/http.cpp (line 339)
https://reviews.apache.org/r/36720/#comment147323

complexity of a function is measured by the nested if condition in it. 
Maybe we accomplish the same using simple small functions ?


- Jojy Varghese


On July 25, 2015, 2:32 p.m., Anand Mazumdar wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36720/
 ---
 
 (Updated July 25, 2015, 2:32 p.m.)
 
 
 Review request for mesos and Ben Mahler.
 
 
 Bugs: MESOS-2294
 https://issues.apache.org/jira/browse/MESOS-2294
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Split review out of r36318. This change adds the functionality of making a 
 http call for subscribe and the master responding with a subscribed event on 
 the persistent stream.
 
 
 Diffs
 -
 
   src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 
   src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 
   src/master/master.cpp 613a011e205611702921179b6c436d62447e2dca 
   src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac 
 
 Diff: https://reviews.apache.org/r/36720/diff/
 
 
 Testing
 ---
 
 make check + test.
 
 
 Thanks,
 
 Anand Mazumdar
 




Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-07-26 Thread Jojy Varghese

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/#review93048
---



src/master/http.cpp (line 359)
https://reviews.apache.org/r/36720/#comment147318

Why not static initialization pattern :
OK ok = {
   .type = Response::PIPE,
   .reader = pipe.reader(),
   };
   
Maybe its not allowed in  the coding style but I would think it looks 
elegant.


- Jojy Varghese


On July 25, 2015, 2:32 p.m., Anand Mazumdar wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36720/
 ---
 
 (Updated July 25, 2015, 2:32 p.m.)
 
 
 Review request for mesos and Ben Mahler.
 
 
 Bugs: MESOS-2294
 https://issues.apache.org/jira/browse/MESOS-2294
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Split review out of r36318. This change adds the functionality of making a 
 http call for subscribe and the master responding with a subscribed event on 
 the persistent stream.
 
 
 Diffs
 -
 
   src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 
   src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 
   src/master/master.cpp 613a011e205611702921179b6c436d62447e2dca 
   src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac 
 
 Diff: https://reviews.apache.org/r/36720/diff/
 
 
 Testing
 ---
 
 make check + test.
 
 
 Thanks,
 
 Anand Mazumdar
 




Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-07-25 Thread Anand Mazumdar

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/
---

(Updated July 25, 2015, 7:26 a.m.)


Review request for mesos and Ben Mahler.


Bugs: MESOS-2294
https://issues.apache.org/jira/browse/MESOS-2294


Repository: mesos


Description
---

Split review out of r36318. This change adds the functionality of making a http 
call for subscribe and the master responding with a subscribed event on the 
persistent stream.


Diffs
-

  src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 
  src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 
  src/master/master.cpp 613a011e205611702921179b6c436d62447e2dca 
  src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac 

Diff: https://reviews.apache.org/r/36720/diff/


Testing
---

make check + test.


Thanks,

Anand Mazumdar



Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-07-25 Thread Anand Mazumdar

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/
---

(Updated July 25, 2015, 7:42 a.m.)


Review request for mesos and Ben Mahler.


Bugs: MESOS-2294
https://issues.apache.org/jira/browse/MESOS-2294


Repository: mesos


Description
---

Split review out of r36318. This change adds the functionality of making a http 
call for subscribe and the master responding with a subscribed event on the 
persistent stream.


Diffs (updated)
-

  src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 
  src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 
  src/master/master.cpp 613a011e205611702921179b6c436d62447e2dca 
  src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac 

Diff: https://reviews.apache.org/r/36720/diff/


Testing
---

make check + test.


Thanks,

Anand Mazumdar



Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-07-25 Thread Mesos ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/#review93022
---


Bad patch!

Reviews applied: [36733, 36717, 36318]

Failed command: ./support/apply-review.sh -n -r 36318

Error:
 2015-07-25 07:28:15 URL:https://reviews.apache.org/r/36318/diff/raw/ 
[23397/23397] - 36318.patch [1]
error: patch failed: src/master/master.hpp:89
error: src/master/master.hpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On July 25, 2015, 7:26 a.m., Anand Mazumdar wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36720/
 ---
 
 (Updated July 25, 2015, 7:26 a.m.)
 
 
 Review request for mesos and Ben Mahler.
 
 
 Bugs: MESOS-2294
 https://issues.apache.org/jira/browse/MESOS-2294
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Split review out of r36318. This change adds the functionality of making a 
 http call for subscribe and the master responding with a subscribed event on 
 the persistent stream.
 
 
 Diffs
 -
 
   src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 
   src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 
   src/master/master.cpp 613a011e205611702921179b6c436d62447e2dca 
   src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac 
 
 Diff: https://reviews.apache.org/r/36720/diff/
 
 
 Testing
 ---
 
 make check + test.
 
 
 Thanks,
 
 Anand Mazumdar
 




Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-07-25 Thread Anand Mazumdar

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/
---

(Updated July 25, 2015, 2:32 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

*Trigger reviewbot again*


Bugs: MESOS-2294
https://issues.apache.org/jira/browse/MESOS-2294


Repository: mesos


Description
---

Split review out of r36318. This change adds the functionality of making a http 
call for subscribe and the master responding with a subscribed event on the 
persistent stream.


Diffs
-

  src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 
  src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 
  src/master/master.cpp 613a011e205611702921179b6c436d62447e2dca 
  src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac 

Diff: https://reviews.apache.org/r/36720/diff/


Testing
---

make check + test.


Thanks,

Anand Mazumdar



Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-07-25 Thread Mesos ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/#review93033
---


Patch looks great!

Reviews applied: [36733, 36717, 36318, 36720]

All tests passed.

- Mesos ReviewBot


On July 25, 2015, 2:32 p.m., Anand Mazumdar wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36720/
 ---
 
 (Updated July 25, 2015, 2:32 p.m.)
 
 
 Review request for mesos and Ben Mahler.
 
 
 Bugs: MESOS-2294
 https://issues.apache.org/jira/browse/MESOS-2294
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Split review out of r36318. This change adds the functionality of making a 
 http call for subscribe and the master responding with a subscribed event on 
 the persistent stream.
 
 
 Diffs
 -
 
   src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 
   src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 
   src/master/master.cpp 613a011e205611702921179b6c436d62447e2dca 
   src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac 
 
 Diff: https://reviews.apache.org/r/36720/diff/
 
 
 Testing
 ---
 
 make check + test.
 
 
 Thanks,
 
 Anand Mazumdar
 




Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-07-25 Thread Anand Mazumdar

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/
---

(Updated July 25, 2015, 7 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

Moved the tests to use the decoder from MESOS-3067.


Bugs: MESOS-2294
https://issues.apache.org/jira/browse/MESOS-2294


Repository: mesos


Description
---

Split review out of r36318. This change adds the functionality of making a http 
call for subscribe and the master responding with a subscribed event on the 
persistent stream.


Diffs (updated)
-

  src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 
  src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 
  src/master/master.cpp 613a011e205611702921179b6c436d62447e2dca 
  src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac 

Diff: https://reviews.apache.org/r/36720/diff/


Testing
---

make check + test.


Thanks,

Anand Mazumdar



Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-07-25 Thread Mesos ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/#review93021
---


Bad patch!

Reviews applied: [36733]

Failed command: ./support/apply-review.sh -n -r 36733

Error:
 2015-07-25 07:07:12 URL:https://reviews.apache.org/r/36733/diff/raw/ 
[58421/58421] - 36733.patch [1]
error: patch failed: src/master/master.hpp:901
error: src/master/master.hpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On July 25, 2015, 7 a.m., Anand Mazumdar wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36720/
 ---
 
 (Updated July 25, 2015, 7 a.m.)
 
 
 Review request for mesos and Ben Mahler.
 
 
 Bugs: MESOS-2294
 https://issues.apache.org/jira/browse/MESOS-2294
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Split review out of r36318. This change adds the functionality of making a 
 http call for subscribe and the master responding with a subscribed event on 
 the persistent stream.
 
 
 Diffs
 -
 
   src/master/http.cpp 3a1598fad4db03e5f62fd4a6bd26b2bedeee4070 
   src/master/master.hpp 827d0d599912b2936beb9615610f627f6c9a2d43 
   src/master/master.cpp 613a011e205611702921179b6c436d62447e2dca 
   src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac 
 
 Diff: https://reviews.apache.org/r/36720/diff/
 
 
 Testing
 ---
 
 make check + test.
 
 
 Thanks,
 
 Anand Mazumdar
 




Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-07-23 Thread Mesos ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/#review92728
---


Bad patch!

Reviews applied: [36717, 36318]

Failed command: ./support/apply-review.sh -n -r 36318

Error:
 2015-07-23 06:05:14 URL:https://reviews.apache.org/r/36318/diff/raw/ 
[29019/29019] - 36318.patch [1]
error: patch failed: src/master/master.hpp:1512
error: src/master/master.hpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On July 23, 2015, 4:50 a.m., Anand Mazumdar wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36720/
 ---
 
 (Updated July 23, 2015, 4:50 a.m.)
 
 
 Review request for mesos and Ben Mahler.
 
 
 Bugs: MESOS-2294
 https://issues.apache.org/jira/browse/MESOS-2294
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Split review out of r36318. This change adds the functionality of making a 
 http call for subscribe and the master responding with a subscribed event on 
 the persistent stream.
 
 
 Diffs
 -
 
   src/master/http.cpp 6f5ca02c52462495b84e77525a6c88299746ece2 
   src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac 
 
 Diff: https://reviews.apache.org/r/36720/diff/
 
 
 Testing
 ---
 
 make check + test.
 
 
 Thanks,
 
 Anand Mazumdar
 




Review Request 36720: Add subscribe- subscribed workflow for http frameworks

2015-07-22 Thread Anand Mazumdar

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/
---

Review request for mesos and Ben Mahler.


Bugs: MESOS-2294
https://issues.apache.org/jira/browse/MESOS-2294


Repository: mesos


Description
---

Split review out of r36318. This change adds the functionality of making a http 
call for subscribe and the master responding with a subscribed event on the 
persistent stream.


Diffs
-

  src/master/http.cpp 6f5ca02c52462495b84e77525a6c88299746ece2 
  src/tests/http_api_tests.cpp 64bbeb6699171e85a5be293919ad9f32ded0ebac 

Diff: https://reviews.apache.org/r/36720/diff/


Testing
---

make check + test.


Thanks,

Anand Mazumdar