Re: Review Request 36586: Updated scheduler driver to send SUBSCRIBE call.

2015-07-22 Thread Vinod Kone

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

(Updated July 22, 2015, 6:11 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

updated bug id.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/sched/sched.cpp 25e2d660f4ee4c0b21c887f78ad04819012966f9 
  src/tests/master_tests.cpp 1e934c4b168a0afabd5065e2c8ffa131362ed29b 
  src/tests/rate_limiting_tests.cpp 6a93df086bc0f256c7750d06f950d61f2dfb7b5c 
  src/tests/slave_recovery_tests.cpp de2fc280abae98a5fbbeae6e230a1bfdaf0fc86e 

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


Testing
---

make check


Thanks,

Vinod Kone



Re: Review Request 36586: Updated scheduler driver to send SUBSCRIBE call.

2015-07-22 Thread Vinod Kone


 On July 20, 2015, 9:46 p.m., Ben Mahler wrote:
  src/tests/rate_limiting_tests.cpp, lines 134-136
  https://reviews.apache.org/r/36586/diff/2/?file=1015049#file1015049line134
 
  Note that in these tests, we've lost the fact that some of these 
  subscribe calls should not have a FrameworkID (i.e. register) and some 
  should have a FrameworkID (i.e. re-register). I suppose we were technically 
  not testing this before, but at least we had more confidence given the 
  different types of the messages.
  
  Should we be adding in some expectations for 'FrameworkID' being 
  set/unset appropriately?

If framework id is not set/unset appropriately master should barf and respond 
with FrameworkError, so I think we are ok.


- Vinod


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


On July 17, 2015, 11:08 p.m., Vinod Kone wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36586/
 ---
 
 (Updated July 17, 2015, 11:08 p.m.)
 
 
 Review request for mesos and Ben Mahler.
 
 
 Bugs: MESOS-3055
 https://issues.apache.org/jira/browse/MESOS-3055
 
 
 Repository: mesos
 
 
 Description
 ---
 
 See summary.
 
 
 Diffs
 -
 
   src/sched/sched.cpp 25e2d660f4ee4c0b21c887f78ad04819012966f9 
   src/tests/master_tests.cpp 1e934c4b168a0afabd5065e2c8ffa131362ed29b 
   src/tests/rate_limiting_tests.cpp 6a93df086bc0f256c7750d06f950d61f2dfb7b5c 
   src/tests/slave_recovery_tests.cpp de2fc280abae98a5fbbeae6e230a1bfdaf0fc86e 
 
 Diff: https://reviews.apache.org/r/36586/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Vinod Kone
 




Re: Review Request 36586: Updated scheduler driver to send SUBSCRIBE call.

2015-07-20 Thread Ben Mahler

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

Ship it!


Looks good, just a question around whether we want to be testing for 
'FrameworkID' being set correctly, in order to distinguish subscription from 
re-subscription (yet another time that re-subscription seems to be a useful 
concept ;)).


src/tests/rate_limiting_tests.cpp (lines 134 - 136)
https://reviews.apache.org/r/36586/#comment146421

Note that in these tests, we've lost the fact that some of these subscribe 
calls should not have a FrameworkID (i.e. register) and some should have a 
FrameworkID (i.e. re-register). I suppose we were technically not testing this 
before, but at least we had more confidence given the different types of the 
messages.

Should we be adding in some expectations for 'FrameworkID' being set/unset 
appropriately?


- Ben Mahler


On July 17, 2015, 11:08 p.m., Vinod Kone wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36586/
 ---
 
 (Updated July 17, 2015, 11:08 p.m.)
 
 
 Review request for mesos and Ben Mahler.
 
 
 Bugs: MESOS-3055
 https://issues.apache.org/jira/browse/MESOS-3055
 
 
 Repository: mesos
 
 
 Description
 ---
 
 See summary.
 
 
 Diffs
 -
 
   src/sched/sched.cpp 25e2d660f4ee4c0b21c887f78ad04819012966f9 
   src/tests/master_tests.cpp 1e934c4b168a0afabd5065e2c8ffa131362ed29b 
   src/tests/rate_limiting_tests.cpp 6a93df086bc0f256c7750d06f950d61f2dfb7b5c 
   src/tests/slave_recovery_tests.cpp de2fc280abae98a5fbbeae6e230a1bfdaf0fc86e 
 
 Diff: https://reviews.apache.org/r/36586/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Vinod Kone
 




Re: Review Request 36586: Updated scheduler driver to send SUBSCRIBE call.

2015-07-17 Thread Vinod Kone

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

(Updated July 17, 2015, 11:08 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/sched/sched.cpp 25e2d660f4ee4c0b21c887f78ad04819012966f9 
  src/tests/master_tests.cpp 1e934c4b168a0afabd5065e2c8ffa131362ed29b 
  src/tests/rate_limiting_tests.cpp 6a93df086bc0f256c7750d06f950d61f2dfb7b5c 
  src/tests/slave_recovery_tests.cpp de2fc280abae98a5fbbeae6e230a1bfdaf0fc86e 

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


Testing
---

make check


Thanks,

Vinod Kone



Review Request 36586: Updated scheduler driver to send SUBSCRIBE call.

2015-07-17 Thread Vinod Kone

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/sched/sched.cpp 81637961f3d931a27ac14220409571593d70b112 
  src/tests/master_tests.cpp 9205ec409d10fc9f7e968eed962fd9ea3c33eed5 
  src/tests/rate_limiting_tests.cpp 6a93df086bc0f256c7750d06f950d61f2dfb7b5c 
  src/tests/slave_recovery_tests.cpp de2fc280abae98a5fbbeae6e230a1bfdaf0fc86e 

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


Testing
---

make check


Thanks,

Vinod Kone



Re: Review Request 36586: Updated scheduler driver to send SUBSCRIBE call.

2015-07-17 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [36586]

All tests passed.

- Mesos ReviewBot


On July 17, 2015, 11:08 p.m., Vinod Kone wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36586/
 ---
 
 (Updated July 17, 2015, 11:08 p.m.)
 
 
 Review request for mesos and Ben Mahler.
 
 
 Bugs: MESOS-3055
 https://issues.apache.org/jira/browse/MESOS-3055
 
 
 Repository: mesos
 
 
 Description
 ---
 
 See summary.
 
 
 Diffs
 -
 
   src/sched/sched.cpp 25e2d660f4ee4c0b21c887f78ad04819012966f9 
   src/tests/master_tests.cpp 1e934c4b168a0afabd5065e2c8ffa131362ed29b 
   src/tests/rate_limiting_tests.cpp 6a93df086bc0f256c7750d06f950d61f2dfb7b5c 
   src/tests/slave_recovery_tests.cpp de2fc280abae98a5fbbeae6e230a1bfdaf0fc86e 
 
 Diff: https://reviews.apache.org/r/36586/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Vinod Kone
 




Re: Review Request 36586: Updated scheduler driver to send SUBSCRIBE call.

2015-07-17 Thread Mesos ReviewBot

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


Bad patch!

Reviews applied: [36586]

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

Error:
 2015-07-17 22:15:49 URL:https://reviews.apache.org/r/36586/diff/raw/ 
[19897/19897] - 36586.patch [1]
error: patch failed: src/sched/sched.cpp:662
error: src/sched/sched.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On July 17, 2015, 9:04 p.m., Vinod Kone wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36586/
 ---
 
 (Updated July 17, 2015, 9:04 p.m.)
 
 
 Review request for mesos and Ben Mahler.
 
 
 Bugs: MESOS-3055
 https://issues.apache.org/jira/browse/MESOS-3055
 
 
 Repository: mesos
 
 
 Description
 ---
 
 See summary.
 
 
 Diffs
 -
 
   src/sched/sched.cpp 81637961f3d931a27ac14220409571593d70b112 
   src/tests/master_tests.cpp 9205ec409d10fc9f7e968eed962fd9ea3c33eed5 
   src/tests/rate_limiting_tests.cpp 6a93df086bc0f256c7750d06f950d61f2dfb7b5c 
   src/tests/slave_recovery_tests.cpp de2fc280abae98a5fbbeae6e230a1bfdaf0fc86e 
 
 Diff: https://reviews.apache.org/r/36586/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Vinod Kone