You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Du Li <li...@gmail.com> on 2013/10/26 01:40:16 UTC

Review Request 14960: implementation of CLI mesos-status

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

Review request for mesos and Benjamin Hindman.


Repository: mesos-git


Description
-------

This commit implements the CLI command mesos-status, which reports three categories of hosts:

(1) live -- The hosts that are reported by the master/state.json and responded correctly to monitor/statistics.json query;
(2) dead -- Those that are reported by master but responded incorrectly to query;
(3) not running -- Those that are included in the var/mesos/deploy/slaves files but not reported by the master.

In addition, the two functions ulimit and resolve are extracted from the original mesos-ps file as module cli_utils.py because they could be reused in other CLI commands.


Diffs
-----

  src/cli/cli_utils.py PRE-CREATION 
  src/cli/mesos-ps a6a8e9b 
  src/cli/mesos-status PRE-CREATION 

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


Testing
-------

tested on single-host deployment of mesos.


Thanks,

Du Li


Re: Review Request 14960: implementation of CLI mesos-status

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14960/#review28720
-----------------------------------------------------------


The diff on this one shows the file already exists?

- Ben Mahler


On Nov. 1, 2013, 3:14 a.m., Du Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14960/
> -----------------------------------------------------------
> 
> (Updated Nov. 1, 2013, 3:14 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Shingo Omura, and Niklas Nielsen.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> This commit implements the CLI command mesos-status, which reports three categories of hosts:
> 
> (1) live -- The hosts that are reported by the master/state.json and responded correctly to monitor/statistics.json query;
> (2) dead -- Those that are reported by master but responded incorrectly to query;
> (3) not running -- Those that are included in the var/mesos/deploy/slaves files but not reported by the master.
> 
> In addition, the two functions ulimit and resolve are extracted from the original mesos-ps file as module cli_utils.py because they could be reused in other CLI commands.
> 
> 
> Diffs
> -----
> 
>   src/cli/cli_utils.py PRE-CREATION 
>   src/cli/mesos-status PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/14960/diff/
> 
> 
> Testing
> -------
> 
> tested on single-host deployment of mesos.
> 
> 
> Thanks,
> 
> Du Li
> 
>


Re: Review Request 14960: implementation of CLI mesos-status

Posted by Shingo Omura <ev...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14960/#review28048
-----------------------------------------------------------



src/cli/cli_utils.py
<https://reviews.apache.org/r/14960/#comment54556>

    python version check used in mesos-ps(https://reviews.apache.org/r/14941/) would be required.


- Shingo Omura


On Nov. 1, 2013, 3:14 a.m., Du Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14960/
> -----------------------------------------------------------
> 
> (Updated Nov. 1, 2013, 3:14 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Shingo Omura, and Niklas Nielsen.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> This commit implements the CLI command mesos-status, which reports three categories of hosts:
> 
> (1) live -- The hosts that are reported by the master/state.json and responded correctly to monitor/statistics.json query;
> (2) dead -- Those that are reported by master but responded incorrectly to query;
> (3) not running -- Those that are included in the var/mesos/deploy/slaves files but not reported by the master.
> 
> In addition, the two functions ulimit and resolve are extracted from the original mesos-ps file as module cli_utils.py because they could be reused in other CLI commands.
> 
> 
> Diffs
> -----
> 
>   src/cli/cli_utils.py PRE-CREATION 
>   src/cli/mesos-status PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/14960/diff/
> 
> 
> Testing
> -------
> 
> tested on single-host deployment of mesos.
> 
> 
> Thanks,
> 
> Du Li
> 
>


Re: Review Request 14960: implementation of CLI mesos-status

Posted by Du Li <li...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14960/#review28983
-----------------------------------------------------------


The cli_utils.py file is no longer needed and hence not included in the new diff.

- Du Li


On Nov. 15, 2013, 7:31 p.m., Du Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14960/
> -----------------------------------------------------------
> 
> (Updated Nov. 15, 2013, 7:31 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Shingo Omura.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> This commit implements the CLI command mesos-status, which reports three categories of hosts:
> 
> (1) The hosts that are reported by the /master/state.json and responded to /slave(1)/health query;
> (2) Those that are reported by master but failed to respond to health query by timeout;
> (3) Those that are included in the var/mesos/deploy/slaves files but not reported by the master.
> 
> 
> Diffs
> -----
> 
>   src/cli/mesos-status PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/14960/diff/
> 
> 
> Testing
> -------
> 
> tested on single-host deployment of mesos.
> 
> 
> Thanks,
> 
> Du Li
> 
>


Re: Review Request 14960: implementation of CLI mesos-status

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14960/#review87790
-----------------------------------------------------------


Patch looks great!

Reviews applied: [14960]

All tests passed.

- Mesos ReviewBot


On Nov. 19, 2013, 11:47 p.m., Du Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14960/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2013, 11:47 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Shingo Omura.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This commit implements the CLI command mesos-status, which reports three categories of hosts:
> 
> (1) The hosts that are reported by the /master/state.json and responded to /slave(1)/health query;
> (2) Those that are reported by master but failed to respond to health query by timeout;
> (3) Those that are included in the var/mesos/deploy/slaves files but not reported by the master.
> 
> 
> Diffs
> -----
> 
>   src/cli/mesos-status PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/14960/diff/
> 
> 
> Testing
> -------
> 
> has been tested on a local cluster of 12 servers.
> 
> 
> Thanks,
> 
> Du Li
> 
>


Re: Review Request 14960: implementation of CLI mesos-status

Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14960/#review87773
-----------------------------------------------------------


Hi Du!

I am sorry this ended up being a stale reviewl: If you still want it in, would you mind addressing Shingo's comment and run a pep8 style checker?

- Niklas Nielsen


On Nov. 19, 2013, 3:47 p.m., Du Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14960/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2013, 3:47 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Shingo Omura.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This commit implements the CLI command mesos-status, which reports three categories of hosts:
> 
> (1) The hosts that are reported by the /master/state.json and responded to /slave(1)/health query;
> (2) Those that are reported by master but failed to respond to health query by timeout;
> (3) Those that are included in the var/mesos/deploy/slaves files but not reported by the master.
> 
> 
> Diffs
> -----
> 
>   src/cli/mesos-status PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/14960/diff/
> 
> 
> Testing
> -------
> 
> has been tested on a local cluster of 12 servers.
> 
> 
> Thanks,
> 
> Du Li
> 
>


Re: Review Request 14960: implementation of CLI mesos-status

Posted by Du Li <li...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14960/
-----------------------------------------------------------

(Updated Nov. 19, 2013, 11:47 p.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, and Shingo Omura.


Changes
-------

This commit implements the CLI command mesos-status, which reports three categories of hosts:

(1) The hosts that are reported by the /master/state.json and responded to /slave(1)/health query;
(2) Those that are reported by master but failed to respond to health query by timeout.

This diff is rebased on latest code on master and has temporarily removed code for checking configuration file.


Repository: mesos-git


Description
-------

This commit implements the CLI command mesos-status, which reports three categories of hosts:

(1) The hosts that are reported by the /master/state.json and responded to /slave(1)/health query;
(2) Those that are reported by master but failed to respond to health query by timeout;
(3) Those that are included in the var/mesos/deploy/slaves files but not reported by the master.


Diffs (updated)
-----

  src/cli/mesos-status PRE-CREATION 

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


Testing
-------

has been tested on a local cluster of 12 servers.


Thanks,

Du Li


Re: Review Request 14960: implementation of CLI mesos-status

Posted by Du Li <li...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14960/
-----------------------------------------------------------

(Updated Nov. 19, 2013, 11:43 p.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, and Shingo Omura.


Changes
-------

This commit implements the CLI command mesos-status, which reports three categories of hosts:

(1) The hosts that are reported by the /master/state.json and responded to /slave(1)/health query;
(2) Those that are reported by master but failed to respond to health query by timeout.


Repository: mesos-git


Description
-------

This commit implements the CLI command mesos-status, which reports three categories of hosts:

(1) The hosts that are reported by the /master/state.json and responded to /slave(1)/health query;
(2) Those that are reported by master but failed to respond to health query by timeout;
(3) Those that are included in the var/mesos/deploy/slaves files but not reported by the master.


Diffs (updated)
-----

  src/cli/mesos-status PRE-CREATION 

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


Testing
-------

has been tested on a local cluster of 12 servers.


Thanks,

Du Li


Re: Review Request 14960: implementation of CLI mesos-status

Posted by Du Li <li...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14960/
-----------------------------------------------------------

(Updated Nov. 16, 2013, 1:12 a.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, and Shingo Omura.


Changes
-------

This commit implements the CLI command mesos-status, which reports three categories of hosts:

(1) The hosts that are reported by the /master/state.json and responded to /slave(1)/health query;
(2) Those that are reported by master but failed to respond to health query by timeout;
(3) Those that are included in the var/mesos/deploy/slaves files but not reported by the master.

Here in this new diff a bug was fixed that calls mesos.cli.usage() with only one argument. Replaced with an error() same as defined in mesos-upload.


Repository: mesos-git


Description
-------

This commit implements the CLI command mesos-status, which reports three categories of hosts:

(1) The hosts that are reported by the /master/state.json and responded to /slave(1)/health query;
(2) Those that are reported by master but failed to respond to health query by timeout;
(3) Those that are included in the var/mesos/deploy/slaves files but not reported by the master.


Diffs (updated)
-----

  src/cli/mesos-status PRE-CREATION 

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


Testing (updated)
-------

has been tested on a local cluster of 12 servers.


Thanks,

Du Li


Re: Review Request 14960: implementation of CLI mesos-status

Posted by Du Li <li...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14960/
-----------------------------------------------------------

(Updated Nov. 15, 2013, 7:31 p.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, and Shingo Omura.


Changes
-------

This implementation of mesos-status uses futures to health-check slaves reported by master/state.json. In addition, it allows for empty and comment lines in the deploy/slaves configuration file.


Repository: mesos-git


Description (updated)
-------

This commit implements the CLI command mesos-status, which reports three categories of hosts:

(1) The hosts that are reported by the /master/state.json and responded to /slave(1)/health query;
(2) Those that are reported by master but failed to respond to health query by timeout;
(3) Those that are included in the var/mesos/deploy/slaves files but not reported by the master.


Diffs (updated)
-----

  src/cli/mesos-status PRE-CREATION 

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


Testing
-------

tested on single-host deployment of mesos.


Thanks,

Du Li


Re: Review Request 14960: implementation of CLI mesos-status

Posted by Du Li <li...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14960/
-----------------------------------------------------------

(Updated Nov. 1, 2013, 3:14 a.m.)


Review request for mesos, Benjamin Hindman, Shingo Omura, and Niklas Nielsen.


Changes
-------

This diff mainly addresses the style fixes as recommended by Niklas Nielsen. In addition, it also incorporates a few python version related fixes by Shingo Omura.


Repository: mesos-git


Description
-------

This commit implements the CLI command mesos-status, which reports three categories of hosts:

(1) live -- The hosts that are reported by the master/state.json and responded correctly to monitor/statistics.json query;
(2) dead -- Those that are reported by master but responded incorrectly to query;
(3) not running -- Those that are included in the var/mesos/deploy/slaves files but not reported by the master.

In addition, the two functions ulimit and resolve are extracted from the original mesos-ps file as module cli_utils.py because they could be reused in other CLI commands.


Diffs (updated)
-----

  src/cli/cli_utils.py PRE-CREATION 
  src/cli/mesos-status PRE-CREATION 

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


Testing
-------

tested on single-host deployment of mesos.


Thanks,

Du Li


Re: Review Request 14960: implementation of CLI mesos-status

Posted by Du Li <li...@gmail.com>.

> On Oct. 31, 2013, 1:20 a.m., Niklas Nielsen wrote:
> > Just a few style/white space comments.

Thanks, Niklas. I will submit the fixes soon.


- Du


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


On Oct. 25, 2013, 11:40 p.m., Du Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14960/
> -----------------------------------------------------------
> 
> (Updated Oct. 25, 2013, 11:40 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> This commit implements the CLI command mesos-status, which reports three categories of hosts:
> 
> (1) live -- The hosts that are reported by the master/state.json and responded correctly to monitor/statistics.json query;
> (2) dead -- Those that are reported by master but responded incorrectly to query;
> (3) not running -- Those that are included in the var/mesos/deploy/slaves files but not reported by the master.
> 
> In addition, the two functions ulimit and resolve are extracted from the original mesos-ps file as module cli_utils.py because they could be reused in other CLI commands.
> 
> 
> Diffs
> -----
> 
>   src/cli/cli_utils.py PRE-CREATION 
>   src/cli/mesos-ps a6a8e9b 
>   src/cli/mesos-status PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/14960/diff/
> 
> 
> Testing
> -------
> 
> tested on single-host deployment of mesos.
> 
> 
> Thanks,
> 
> Du Li
> 
>


Re: Review Request 14960: implementation of CLI mesos-status

Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14960/#review27842
-----------------------------------------------------------


Just a few style/white space comments.


src/cli/cli_utils.py
<https://reviews.apache.org/r/14960/#comment54195>

    Kill new line?



src/cli/cli_utils.py
<https://reviews.apache.org/r/14960/#comment54187>

    Usual style is TODO(<your handle>): <comment>.
    
    So it becomes: 
    
    TODO(duli): to be cleaned up further, e.g., sys.exit should perhaps not be used.



src/cli/cli_utils.py
<https://reviews.apache.org/r/14960/#comment54188>

    Can you kill white space?



src/cli/mesos-status
<https://reviews.apache.org/r/14960/#comment54190>

    Kill white space



src/cli/mesos-status
<https://reviews.apache.org/r/14960/#comment54206>

    Kill this line if it is not needed?



src/cli/mesos-status
<https://reviews.apache.org/r/14960/#comment54192>

    Kill white space



src/cli/mesos-status
<https://reviews.apache.org/r/14960/#comment54201>

    s/dict/dictionary/ ?
    
    Words are usually spelled out.



src/cli/mesos-status
<https://reviews.apache.org/r/14960/#comment54203>

    Comment on its own line.



src/cli/mesos-status
<https://reviews.apache.org/r/14960/#comment54193>

    Kill trailing white space


- Niklas Nielsen


On Oct. 25, 2013, 11:40 p.m., Du Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14960/
> -----------------------------------------------------------
> 
> (Updated Oct. 25, 2013, 11:40 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> This commit implements the CLI command mesos-status, which reports three categories of hosts:
> 
> (1) live -- The hosts that are reported by the master/state.json and responded correctly to monitor/statistics.json query;
> (2) dead -- Those that are reported by master but responded incorrectly to query;
> (3) not running -- Those that are included in the var/mesos/deploy/slaves files but not reported by the master.
> 
> In addition, the two functions ulimit and resolve are extracted from the original mesos-ps file as module cli_utils.py because they could be reused in other CLI commands.
> 
> 
> Diffs
> -----
> 
>   src/cli/cli_utils.py PRE-CREATION 
>   src/cli/mesos-ps a6a8e9b 
>   src/cli/mesos-status PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/14960/diff/
> 
> 
> Testing
> -------
> 
> tested on single-host deployment of mesos.
> 
> 
> Thanks,
> 
> Du Li
> 
>