Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-04-06 Thread Kapil Arya 

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


Ship it!




Ship It!

- Kapil Arya


On April 6, 2016, 12:02 p.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44287/
> ---
> 
> (Updated April 6, 2016, 12:02 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MasterContender and MasterDetector abstract classes can be subclassed by
> user-provided classes to allow for arbitrary contender/detector 
> implementations.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/contender.hpp PRE-CREATION 
>   include/mesos/master/detector.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44287/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/44670/.
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-04-06 Thread Anurag Singh 

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

(Updated April 6, 2016, 4:02 p.m.)


Review request for mesos, Benjamin Hindman and Kapil Arya.


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


Repository: mesos


Description
---

MasterContender and MasterDetector abstract classes can be subclassed by
user-provided classes to allow for arbitrary contender/detector implementations.


Diffs (updated)
-

  include/mesos/master/contender.hpp PRE-CREATION 
  include/mesos/master/detector.hpp PRE-CREATION 

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


Testing
---

See https://reviews.apache.org/r/44670/.


Thanks,

Anurag Singh

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-04-06 Thread Anurag Singh 

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

(Updated April 6, 2016, 3:13 p.m.)


Review request for mesos, Benjamin Hindman and Kapil Arya.


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


Repository: mesos


Description
---

MasterContender and MasterDetector abstract classes can be subclassed by
user-provided classes to allow for arbitrary contender/detector implementations.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/errorbase.hpp 
96d395d37cdad706c2f23fdd9caeefc2a33e0541 
  3rdparty/libprocess/3rdparty/stout/include/stout/flags/parse.hpp 
ef365e4d714a2c25d358a702722c5f1216869382 
  3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/grp.hpp 
232588c46c8207c6a002f60d00aa675f9e505083 
  3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/pwd.hpp 
46e3ec25cbf379fb2b82297849216b0866baf333 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
edaa76a5322d0bf60b7172405aa754b5aca95458 
  3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp 
89dedec0c818263f17f220a2e71ed470bf75a42f 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp 
d634491b301632ce6468f86d04f21fa25afbbaaa 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
c48106e5905e3be0faeba7177ef534766089faff 
  3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp 
66e16abe914e2a1ee7599bab857ff478b7ec20dc 
  3rdparty/libprocess/include/process/network.hpp 
2bc940fb3ba2bc6b2fed281a3920d13fdb1277e9 
  3rdparty/libprocess/src/encoder.hpp 69163830eaa9f77132a16fc14351b309144827bf 
  3rdparty/libprocess/src/poll_socket.cpp 
cb2878565a112017b190b4ff83dc65a876ea45f9 
  3rdparty/libprocess/src/time.cpp a6c3f3de69e056544406f4416c2d0aea06adc34d 
  CHANGELOG 4553465cc3dc17956f168469d405f7a453d6359e 
  docs/app-framework-development-guide.md 
c5badd00121e7d1752dd2a4b471f97b80ef07926 
  docs/configuration.md 309a5a05eab386c8943ba6bdee8d5efeb448aa0c 
  docs/fetcher.md fd6d8a78bd35c5644dceff7005dd7dfd9f5f2171 
  include/mesos/docker/spec.hpp 2ebacc70d92a593c8dd006b34519c3a2a5225481 
  include/mesos/master/contender.hpp PRE-CREATION 
  include/mesos/master/detector.hpp PRE-CREATION 
  include/mesos/scheduler.hpp 5b153d2acb705cc54fcca86914af72d8b2e3fb55 
  src/Makefile.am 55d3b341361bed25f3aa966d77060c88be29e5b0 
  src/docker/docker.hpp da6e9a25b99079940958001128ee949cdb9b931b 
  src/docker/docker.cpp bb9ddde27464aa4d9215e3ca673fa87c6e00e326 
  src/docker/spec.cpp ac28331a17edb8c2ff81d5a2f79a794f869a3e5d 
  src/java/src/org/apache/mesos/SchedulerDriver.java 
3f03e91b8d65bca725f8063eca54e362a640e298 
  src/launcher/executor.cpp 4658b9e5bb48b09e403991c8bdfdbf4d4dae0416 
  src/linux/cgroups.hpp 5f4010734ed9e3295dcc3a4390123e4f4ce99c16 
  src/linux/fs.cpp 2087b4ac1503e0fd085319b1017389f1f947536f 
  src/python/interface/src/mesos/interface/__init__.py 
1da76ebe577639e8161b16a48a503aa76d568789 
  src/slave/containerizer/docker.cpp 0576eab65a1f3556644472d5c9baba4ea9c34346 
  src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp 
703e49f86ae41c11ae350ae51107eda00d0d5b7e 
  src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
654137c552a7c416f394365e43ea80770fe1ef8d 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
f43ab071de42de16d330cfd2a3b76563bcb173c9 
  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
21a86e3087bea46a2d2125e53292251243086554 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
4f3f210b8d0ab9a453ab56c5e23024e2ab7c4259 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
6545a6d29cb91d6cf8919c7796c283084178e499 
  src/slave/flags.hpp 69e1b01e09d2a15bee5e0745b751f47aaefe3fbe 
  src/slave/flags.cpp 315cf47d268bce0a0255a061d64e414c736c8125 
  src/tests/containerizer/docker_spec_tests.cpp 
796b020f58f8451362bc1357ab6d7ceb4e946b3c 
  src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
5565227bc0b18f1891265e6eff0c5a22a0c4ab86 
  src/tests/containerizer/port_mapping_tests.cpp 
ce985f44cffd74a853154309397b8ee596934f78 
  src/tests/environment.cpp acadb5bb0a1b1a9b0cee0345035b93147bf7164c 
  src/tests/mesos.hpp 3b565b45f45c84aba42aa6fb29b21f8306c49861 
  src/tests/mesos.cpp cf38dbb05908800b3a771318fa6922548c86c1f2 
  support/test-upgrade.py 2c4061d71338f66e432dfa4ac86a9693f3ad38bf 

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


Testing
---

See https://reviews.apache.org/r/44670/.


Thanks,

Anurag Singh

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-04-04 Thread Anurag Singh 

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

(Updated April 4, 2016, 5:07 p.m.)


Review request for mesos, Benjamin Hindman and Kapil Arya.


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


Repository: mesos


Description
---

MasterContender and MasterDetector abstract classes can be subclassed by
user-provided classes to allow for arbitrary contender/detector implementations.


Diffs (updated)
-

  include/mesos/master/contender.hpp PRE-CREATION 
  include/mesos/master/detector.hpp PRE-CREATION 

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


Testing
---

See https://reviews.apache.org/r/44670/.


Thanks,

Anurag Singh

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-03-31 Thread Anurag Singh 


> On March 31, 2016, 5:25 p.m., Kapil Arya wrote:
> > include/mesos/master/contender.hpp, line 65
> > 
> >
> > I am wondering if `type` can be replaced with `moduleName` or something 
> > more explicit.

as discussed, we'll be dropping this method so I'll skip this comment.


- Anurag


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


On March 23, 2016, 11:04 p.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44287/
> ---
> 
> (Updated March 23, 2016, 11:04 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MasterContender and MasterDetector abstract classes can be subclassed by
> user-provided classes to allow for arbitrary contender/detector 
> implementations.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/contender.hpp PRE-CREATION 
>   include/mesos/master/detector.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44287/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/44670/.
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-03-31 Thread Kapil Arya 

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




include/mesos/master/contender.hpp (line 19)


Please include header for `std::string`.



include/mesos/master/contender.hpp (lines 26 - 27)


Alphabetize please!

Also #include stout/try.hpp?



include/mesos/master/contender.hpp (line 65)


I am wondering if `type` can be replaced with `moduleName` or something 
more explicit.



include/mesos/master/detector.hpp (line 62)


Same as with contender, can we replace `type` with a more explicit name?


- Kapil Arya


On March 23, 2016, 7:04 p.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44287/
> ---
> 
> (Updated March 23, 2016, 7:04 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MasterContender and MasterDetector abstract classes can be subclassed by
> user-provided classes to allow for arbitrary contender/detector 
> implementations.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/contender.hpp PRE-CREATION 
>   include/mesos/master/detector.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44287/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/44670/.
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-03-21 Thread Anurag Singh 

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

(Updated March 21, 2016, 3:43 p.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

MasterContender and MasterDetector abstract classes can be subclassed by
user-provided classes to allow for arbitrary contender/detector implementations.


Diffs (updated)
-

  include/mesos/master/contender.hpp PRE-CREATION 
  include/mesos/master/detector.hpp PRE-CREATION 

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


Testing
---

See https://reviews.apache.org/r/44670/.


Thanks,

Anurag Singh

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-03-20 Thread Anurag Singh 


> On March 11, 2016, 9:40 p.m., Joseph Wu wrote:
> > I think you should send an email to the user and dev mailing lists to ask 
> > for high-level feedback on this interface.  We want to make sure the 
> > interface is broad enough to support different implementations.  (And I'm 
> > no expert on leader election.)
> 
> Anurag Singh wrote:
> I'll do that and address your comments.

I haven't seen any comments from the user and developer communities to my email 
on these changes. Do you want to wait longer?


- Anurag


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


On March 15, 2016, 11:01 p.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44287/
> ---
> 
> (Updated March 15, 2016, 11:01 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MasterContender and MasterDetector abstract classes can be subclassed by
> user-provided classes to allow for arbitrary contender/detector 
> implementations.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/contender.hpp PRE-CREATION 
>   include/mesos/master/detector.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44287/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/44670/.
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-03-19 Thread Anurag Singh 

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

(Updated March 18, 2016, 12:29 a.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

MasterContender and MasterDetector abstract classes can be subclassed by
user-provided classes to allow for arbitrary contender/detector implementations.


Diffs (updated)
-

  include/mesos/master/contender.hpp PRE-CREATION 
  include/mesos/master/detector.hpp PRE-CREATION 

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


Testing
---

See https://reviews.apache.org/r/44670/.


Thanks,

Anurag Singh

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-03-19 Thread Joseph Wu 

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



Sorry for the delay...

As Vinod suggested, you might want to consider reaching out to another shepherd 
(as far as I can tell, BenH is not likely to have time in the foreseeable 
future).  (And in case you don't know, I'm not a shepherd :)


include/mesos/master/contender.hpp (line 44)


Nit: Use backticks instead of single-quotes for references to the code in 
comments.

Ditto for flags and objects (like `ZookeeperMasterContender`)



include/mesos/master/contender.hpp (line 45)


Note that flags are not limited to "command line invocations".  For 
example, you can specify an environment variable `MESOS_ZK=...` to get the same 
effect as `--zk`.



include/mesos/master/contender.hpp (lines 63 - 64)


I'd take out the bit about "key-value pairs".  Since the `Parameters` are 
parsed as protobuf, the JSON ends up looking like:
```
"parameters" : [
  {
"key" : "foo",
"value" : "bar"
  }, {
"key" : "other_key",
"value" : "other_value"
  }
]
```

Also, extra newline.


- Joseph Wu


On March 17, 2016, 5:29 p.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44287/
> ---
> 
> (Updated March 17, 2016, 5:29 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MasterContender and MasterDetector abstract classes can be subclassed by
> user-provided classes to allow for arbitrary contender/detector 
> implementations.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/contender.hpp PRE-CREATION 
>   include/mesos/master/detector.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44287/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/44670/.
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-03-18 Thread Anurag Singh 


> On March 18, 2016, 9:01 p.m., Joseph Wu wrote:
> > Sorry for the delay...
> > 
> > As Vinod suggested, you might want to consider reaching out to another 
> > shepherd (as far as I can tell, BenH is not likely to have time in the 
> > foreseeable future).  (And in case you don't know, I'm not a shepherd :)

Will do.


- Anurag


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


On March 18, 2016, 12:29 a.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44287/
> ---
> 
> (Updated March 18, 2016, 12:29 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MasterContender and MasterDetector abstract classes can be subclassed by
> user-provided classes to allow for arbitrary contender/detector 
> implementations.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/contender.hpp PRE-CREATION 
>   include/mesos/master/detector.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44287/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/44670/.
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-03-15 Thread Anurag Singh 

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

(Updated March 15, 2016, 11:01 p.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

MasterContender and MasterDetector abstract classes can be subclassed by
user-provided classes to allow for arbitrary contender/detector implementations.


Diffs
-

  include/mesos/master/contender.hpp PRE-CREATION 
  include/mesos/master/detector.hpp PRE-CREATION 

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


Testing (updated)
---

See https://reviews.apache.org/r/44670/.


Thanks,

Anurag Singh

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-03-15 Thread Anurag Singh 

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

(Updated March 15, 2016, 6:16 a.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

MasterContender and MasterDetector abstract classes can be subclassed by
user-provided classes to allow for arbitrary contender/detector implementations.


Diffs (updated)
-

  include/mesos/master/contender.hpp PRE-CREATION 
  include/mesos/master/detector.hpp PRE-CREATION 

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


Testing
---

See https://reviews.apache.org/r/44289/.


Thanks,

Anurag Singh

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-03-14 Thread Anurag Singh 

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

(Updated March 14, 2016, 11:48 p.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

MasterContender and MasterDetector abstract classes can be subclassed by
user-provided classes to allow for arbitrary contender/detector implementations.


Diffs (updated)
-

  include/mesos/master/contender.hpp PRE-CREATION 
  include/mesos/master/detector.hpp PRE-CREATION 

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


Testing
---

See https://reviews.apache.org/r/44289/.


Thanks,

Anurag Singh

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-03-11 Thread Anurag Singh 


> On March 11, 2016, 9:40 p.m., Joseph Wu wrote:
> > I think you should send an email to the user and dev mailing lists to ask 
> > for high-level feedback on this interface.  We want to make sure the 
> > interface is broad enough to support different implementations.  (And I'm 
> > no expert on leader election.)

I'll do that and address your comments.


- Anurag


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


On March 10, 2016, 11:45 p.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44287/
> ---
> 
> (Updated March 10, 2016, 11:45 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MasterContender and MasterDetector abstract classes can be subclassed by
> user-provided classes to allow for arbitrary contender/detector 
> implementations.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/contender.hpp PRE-CREATION 
>   include/mesos/master/detector.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44287/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/44289/.
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-03-11 Thread Joseph Wu 

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



I think you should send an email to the user and dev mailing lists to ask for 
high-level feedback on this interface.  We want to make sure the interface is 
broad enough to support different implementations.  (And I'm no expert on 
leader election.)


include/mesos/master/contender.hpp (lines 35 - 37)


Micro-nit: We prefer the java-doc comment style for new public headers.  
You might as well make the change.

```
/**
 * Comment...
 */
```



include/mesos/master/contender.hpp (lines 41 - 47)


Can you expand on these comments a bit?

I think the comments should touch upon:

- `create` exists for backwards compatibility.
- `create` only works for the Standalone and Zookeeper types.  (Also add 
something about the expected input format.)
- `createFromModule` takes a symbol and creates a module.  Any 
configuration for this method should live in the `--modules` flag as a 
`Parameters` object.
- `createFromModule` can return the Standalone type, or any other type 
(assuming you convert the Zookeeper one into an example module).

Ditto for the detector.


- Joseph Wu


On March 10, 2016, 3:45 p.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44287/
> ---
> 
> (Updated March 10, 2016, 3:45 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MasterContender and MasterDetector abstract classes can be subclassed by
> user-provided classes to allow for arbitrary contender/detector 
> implementations.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/contender.hpp PRE-CREATION 
>   include/mesos/master/detector.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44287/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/44289/.
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-03-10 Thread Anurag Singh 

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

(Updated March 10, 2016, 11:45 p.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

MasterContender and MasterDetector abstract classes can be subclassed by
user-provided classes to allow for arbitrary contender/detector implementations.


Diffs (updated)
-

  include/mesos/master/contender.hpp PRE-CREATION 
  include/mesos/master/detector.hpp PRE-CREATION 

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


Testing
---

See https://reviews.apache.org/r/44289/.


Thanks,

Anurag Singh

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-03-10 Thread Anurag Singh 

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

(Updated March 10, 2016, 9:21 p.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

MasterContender and MasterDetector abstract classes can be subclassed by
user-provided classes to allow for arbitrary contender/detector implementations.


Diffs (updated)
-

  include/mesos/master/contender.hpp PRE-CREATION 
  include/mesos/master/detector.hpp PRE-CREATION 

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


Testing
---

See https://reviews.apache.org/r/44289/.


Thanks,

Anurag Singh

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-03-10 Thread Anurag Singh 

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

(Updated March 10, 2016, 7:34 p.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

MasterContender and MasterDetector abstract classes can be subclassed by
user-provided classes to allow for arbitrary contender/detector implementations.


Diffs (updated)
-

  include/mesos/master/contender.hpp PRE-CREATION 
  include/mesos/master/detector.hpp PRE-CREATION 

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


Testing
---

See https://reviews.apache.org/r/44289/.


Thanks,

Anurag Singh

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-03-09 Thread Anurag Singh 

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

(Updated March 9, 2016, 9:20 p.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

MasterContender and MasterDetector abstract classes can be subclassed by
user-provided classes to allow for arbitrary contender/detector implementations.


Diffs (updated)
-

  include/mesos/master/contender.hpp PRE-CREATION 
  include/mesos/master/detector.hpp PRE-CREATION 

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


Testing
---

See https://reviews.apache.org/r/44289/.


Thanks,

Anurag Singh

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-03-08 Thread Anurag Singh 

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

(Updated March 9, 2016, 1:34 a.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

MasterContender and MasterDetector abstract classes can be subclassed by
user-provided classes to allow for arbitrary contender/detector implementations.


Diffs (updated)
-

  include/mesos/master/contender.hpp PRE-CREATION 
  include/mesos/master/detector.hpp PRE-CREATION 

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


Testing
---

See https://reviews.apache.org/r/44289/.


Thanks,

Anurag Singh

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-03-03 Thread Anurag Singh 


> On March 3, 2016, 10:26 p.m., Joseph Wu wrote:
> > include/mesos/master/contender.hpp, line 43
> > 
> >
> > I'm going to reiterate this comment 
> > (https://reviews.apache.org/r/43269/#comment181059).
> > 
> > For modules, the "type" is some string that identifies the entry point 
> > of the entry.  To be backwards compatible and support modules, you will 
> > need either 2 arguments or 2 separate factory methods (or some really ugly 
> > logic inside the factory method).
> 
> Anurag Singh wrote:
> The type is expected to come from the master_contender and 
> master_detector flags from change https://reviews.apache.org/r/44288. So type 
> would be the module name (as you had suggested in 
> https://reviews.apache.org/r/43269/). The parameters (from the JSON set in 
> --modules) are passed to the module by ModuleManager::create. As far as 
> backwards compatibility is concerned, the --zk flag will continue to work as 
> it does now. Can you explain the need for 2 separate arguments?
> 
> Joseph Wu wrote:
> This is necessary for the detector 
> (https://reviews.apache.org/r/44289/diff/3#7.30 ).  There's no guarantee that 
> a module "type" is not parsable as a `UPID`.  (And if we can make this 
> separation explicit in the code, we should.)

That's true. I can add a new method called, say, createFromModule. So if 
master_contender and master_detector are in the argument list, that method 
would be called, instead of create.


- Anurag


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


On March 3, 2016, 5:29 p.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44287/
> ---
> 
> (Updated March 3, 2016, 5:29 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MasterContender and MasterDetector abstract classes can be subclassed by
> user-provided classes to allow for arbitrary contender/detector 
> implementations.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/contender.hpp PRE-CREATION 
>   include/mesos/master/detector.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44287/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/44289/.
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-03-03 Thread Joseph Wu 


> On March 3, 2016, 2:26 p.m., Joseph Wu wrote:
> > include/mesos/master/contender.hpp, line 43
> > 
> >
> > I'm going to reiterate this comment 
> > (https://reviews.apache.org/r/43269/#comment181059).
> > 
> > For modules, the "type" is some string that identifies the entry point 
> > of the entry.  To be backwards compatible and support modules, you will 
> > need either 2 arguments or 2 separate factory methods (or some really ugly 
> > logic inside the factory method).
> 
> Anurag Singh wrote:
> The type is expected to come from the master_contender and 
> master_detector flags from change https://reviews.apache.org/r/44288. So type 
> would be the module name (as you had suggested in 
> https://reviews.apache.org/r/43269/). The parameters (from the JSON set in 
> --modules) are passed to the module by ModuleManager::create. As far as 
> backwards compatibility is concerned, the --zk flag will continue to work as 
> it does now. Can you explain the need for 2 separate arguments?

This is necessary for the detector 
(https://reviews.apache.org/r/44289/diff/3#7.30 ).  There's no guarantee that a 
module "type" is not parsable as a `UPID`.  (And if we can make this separation 
explicit in the code, we should.)


- Joseph


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


On March 3, 2016, 9:29 a.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44287/
> ---
> 
> (Updated March 3, 2016, 9:29 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MasterContender and MasterDetector abstract classes can be subclassed by
> user-provided classes to allow for arbitrary contender/detector 
> implementations.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/contender.hpp PRE-CREATION 
>   include/mesos/master/detector.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44287/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/44289/.
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-03-03 Thread Anurag Singh 


> On March 3, 2016, 10:26 p.m., Joseph Wu wrote:
> > include/mesos/master/contender.hpp, line 43
> > 
> >
> > I'm going to reiterate this comment 
> > (https://reviews.apache.org/r/43269/#comment181059).
> > 
> > For modules, the "type" is some string that identifies the entry point 
> > of the entry.  To be backwards compatible and support modules, you will 
> > need either 2 arguments or 2 separate factory methods (or some really ugly 
> > logic inside the factory method).

The type is expected to come from the master_contender and master_detector 
flags from change https://reviews.apache.org/r/44288. So type would be the 
module name (as you had suggested in https://reviews.apache.org/r/43269/). The 
parameters (from the JSON set in --modules) are passed to the module by 
ModuleManager::create. As far as backwards compatibility is concerned, the --zk 
flag will continue to work as it does now. Can you explain the need for 2 
separate arguments?


- Anurag


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


On March 3, 2016, 5:29 p.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44287/
> ---
> 
> (Updated March 3, 2016, 5:29 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MasterContender and MasterDetector abstract classes can be subclassed by
> user-provided classes to allow for arbitrary contender/detector 
> implementations.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/contender.hpp PRE-CREATION 
>   include/mesos/master/detector.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44287/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/44289/.
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-03-03 Thread Joseph Wu 

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




include/mesos/master/contender.hpp (line 43)


I'm going to reiterate this comment 
(https://reviews.apache.org/r/43269/#comment181059).

For modules, the "type" is some string that identifies the entry point of 
the entry.  To be backwards compatible and support modules, you will need 
either 2 arguments or 2 separate factory methods (or some really ugly logic 
inside the factory method).



include/mesos/master/detector.hpp (line 40)


Same comment as above.


- Joseph Wu


On March 3, 2016, 9:29 a.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44287/
> ---
> 
> (Updated March 3, 2016, 9:29 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MasterContender and MasterDetector abstract classes can be subclassed by
> user-provided classes to allow for arbitrary contender/detector 
> implementations.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/contender.hpp PRE-CREATION 
>   include/mesos/master/detector.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44287/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/44289/.
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-03-03 Thread Anurag Singh 

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

(Updated March 3, 2016, 5:29 p.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

MasterContender and MasterDetector abstract classes can be subclassed by
user-provided classes to allow for arbitrary contender/detector implementations.


Diffs (updated)
-

  include/mesos/master/contender.hpp PRE-CREATION 
  include/mesos/master/detector.hpp PRE-CREATION 

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


Testing
---

See https://reviews.apache.org/r/44289/.


Thanks,

Anurag Singh

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-03-02 Thread Anurag Singh 

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

(Updated March 2, 2016, 9:30 p.m.)


Review request for mesos.


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


Repository: mesos


Description
---

MasterContender and MasterDetector abstract classes can be subclassed by
user-provided classes to allow for arbitrary contender/detector implementations.


Diffs (updated)
-

  include/mesos/master/contender.hpp PRE-CREATION 
  include/mesos/master/detector.hpp PRE-CREATION 

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


Testing
---

See https://reviews.apache.org/r/44289/.


Thanks,

Anurag Singh

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-03-02 Thread Anurag Singh 


> On March 2, 2016, 9:20 p.m., Joseph Wu wrote:
> > Are you working with @mcavage?  (See https://reviews.apache.org/r/43269/)
> > 
> > Also, you might want to add this ticket to your reviews: 
> > https://issues.apache.org/jira/browse/MESOS-4610

That's correct. I'll update this with the ticket.

Based on your suggestions, I've split the change into 3 parts (I couldn't split 
the namespace and refactoring changes because they are too tightly coupled). 
I've also added master_contender and master_detector flags 
(https://reviews.apache.org/r/44288/) so users can specify the modules to use.


- Anurag


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


On March 2, 2016, 9:13 p.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44287/
> ---
> 
> (Updated March 2, 2016, 9:13 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MasterContender and MasterDetector abstract classes can be subclassed by
> user-provided classes to allow for arbitrary contender/detector 
> implementations.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/contender.hpp PRE-CREATION 
>   include/mesos/master/detector.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44287/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/44289/.
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-03-02 Thread Joseph Wu 

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



Are you working with @mcavage?  (See https://reviews.apache.org/r/43269/)

Also, you might want to add this ticket to your reviews: 
https://issues.apache.org/jira/browse/MESOS-4610

- Joseph Wu


On March 2, 2016, 1:13 p.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44287/
> ---
> 
> (Updated March 2, 2016, 1:13 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MasterContender and MasterDetector abstract classes can be subclassed by
> user-provided classes to allow for arbitrary contender/detector 
> implementations.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/contender.hpp PRE-CREATION 
>   include/mesos/master/detector.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44287/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/44289/.
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-03-02 Thread Anurag Singh 

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

Review request for mesos.


Repository: mesos


Description
---

MasterContender and MasterDetector abstract classes can be subclassed by
user-provided classes to allow for arbitrary contender/detector implementations.


Diffs
-

  include/mesos/master/contender.hpp PRE-CREATION 
  include/mesos/master/detector.hpp PRE-CREATION 

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


Testing
---

See https://reviews.apache.org/r/44289/.


Thanks,

Anurag Singh