You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Armand Grillet <ag...@mesosphere.io> on 2017/09/03 08:37:16 UTC

Re: Review Request 61212: Added CLI utility functions to check IPs and ports.

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

(Updated Sept. 3, 2017, 8:37 a.m.)


Review request for mesos and Kevin Klues.


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


Repository: mesos


Description
-------

This will be used by future plugins.


Diffs (updated)
-----

  src/python/cli_new/lib/cli/util.py 7371f83543ed527bea8dbf2fe4e20d92ef8e4492 


Diff: https://reviews.apache.org/r/61212/diff/2/

Changes: https://reviews.apache.org/r/61212/diff/1-2/


Testing
-------

Tested manually, PEP8 and Pylint used to make sure that the code style is correct.


Thanks,

Armand Grillet


Re: Review Request 61212: Added CLI utility functions to check IPs and ports.

Posted by Kevin Klues <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61212/#review185977
-----------------------------------------------------------




src/python/cli_new/lib/cli/util.py
Lines 158 (patched)
<https://reviews.apache.org/r/61212/#comment262324>

    s/check/verify/s
    
    Also, let's leave `name` out of this as a parameter and instead prepend that information by wrapping the error string propogated up.



src/python/cli_new/lib/cli/util.py
Lines 179 (patched)
<https://reviews.apache.org/r/61212/#comment262326>

    get rid of printing host/name here.



src/python/cli_new/lib/cli/util.py
Lines 189 (patched)
<https://reviews.apache.org/r/61212/#comment262325>

    use ip instead of host/name.



src/python/cli_new/lib/cli/util.py
Lines 193 (patched)
<https://reviews.apache.org/r/61212/#comment262329>

    Let's make sure to call this in a loop over all possible masters at the call site. Eventually we will have an `Agent` class that takes a `Master` object that know how to do this looping for us.



src/python/cli_new/lib/cli/util.py
Lines 217 (patched)
<https://reviews.apache.org/r/61212/#comment262330>

    This seems weird. We should just pluck out the first master from the config (what about the other masters in that list?). We should build a `Master` abstraction that takes the config at construction and knows how to loop over all masters when performing any of its operations.



src/python/cli_new/lib/cli/util.py
Lines 218 (patched)
<https://reviews.apache.org/r/61212/#comment262327>

    This should be wrapped in a try. If an error is raised, we can wrap the error produced and print that it ws the result of trying to verify the address elements from a 'master' node.


- Kevin Klues


On Sept. 4, 2017, 2:46 p.m., Armand Grillet wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61212/
> -----------------------------------------------------------
> 
> (Updated Sept. 4, 2017, 2:46 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7840
>     https://issues.apache.org/jira/browse/MESOS-7840
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This will be used by future plugins.
> 
> 
> Diffs
> -----
> 
>   src/python/cli_new/lib/cli/util.py 7371f83543ed527bea8dbf2fe4e20d92ef8e4492 
> 
> 
> Diff: https://reviews.apache.org/r/61212/diff/3/
> 
> 
> Testing
> -------
> 
> Tested manually, PEP8 and Pylint used to make sure that the code style is correct.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>


Re: Review Request 61212: Added CLI utility function to verify addresses.

Posted by Kevin Klues <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61212/#review186254
-----------------------------------------------------------




src/python/cli_new/lib/cli/util.py
Lines 162 (patched)
<https://reviews.apache.org/r/61212/#comment262738>

    Can we change this comment to make the usage of 'basestring' here more clear (how it is the base class for both unicode and string and depending on how this field is passed in it could be either of these).



src/python/cli_new/lib/cli/util.py
Lines 169 (patched)
<https://reviews.apache.org/r/61212/#comment262739>

    s/respect/match/



src/python/cli_new/lib/cli/util.py
Lines 188 (patched)
<https://reviews.apache.org/r/61212/#comment262740>

    Let's break this out into its own commit.


- Kevin Klues


On Sept. 26, 2017, 1:53 p.m., Armand Grillet wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61212/
> -----------------------------------------------------------
> 
> (Updated Sept. 26, 2017, 1:53 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7840
>     https://issues.apache.org/jira/browse/MESOS-7840
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This will be used by future plugins.
> 
> 
> Diffs
> -----
> 
>   src/python/cli_new/lib/cli/util.py 7371f83543ed527bea8dbf2fe4e20d92ef8e4492 
> 
> 
> Diff: https://reviews.apache.org/r/61212/diff/6/
> 
> 
> Testing
> -------
> 
> Tested manually, PEP8 and Pylint used to make sure that the code style is correct.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>


Re: Review Request 61212: Added CLI utility function to verify addresses.

Posted by Armand Grillet <ag...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61212/
-----------------------------------------------------------

(Updated Sept. 26, 2017, 2:15 p.m.)


Review request for mesos and Kevin Klues.


Changes
-------

Resolved issues.


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


Repository: mesos


Description
-------

This will be used by future plugins.


Diffs (updated)
-----

  src/python/cli_new/lib/cli/util.py 7371f83543ed527bea8dbf2fe4e20d92ef8e4492 


Diff: https://reviews.apache.org/r/61212/diff/7/

Changes: https://reviews.apache.org/r/61212/diff/6-7/


Testing
-------

Tested manually, PEP8 and Pylint used to make sure that the code style is correct.


Thanks,

Armand Grillet


Re: Review Request 61212: Added CLI utility function to verify addresses.

Posted by Armand Grillet <ag...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61212/
-----------------------------------------------------------

(Updated Sept. 26, 2017, 1:53 p.m.)


Review request for mesos and Kevin Klues.


Changes
-------

Cut patch.


Summary (updated)
-----------------

Added CLI utility function to verify addresses.


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


Repository: mesos


Description
-------

This will be used by future plugins.


Diffs (updated)
-----

  src/python/cli_new/lib/cli/util.py 7371f83543ed527bea8dbf2fe4e20d92ef8e4492 


Diff: https://reviews.apache.org/r/61212/diff/6/

Changes: https://reviews.apache.org/r/61212/diff/5-6/


Testing
-------

Tested manually, PEP8 and Pylint used to make sure that the code style is correct.


Thanks,

Armand Grillet


Re: Review Request 61212: Added CLI utility functions to verify addresses.

Posted by Armand Grillet <ag...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61212/
-----------------------------------------------------------

(Updated Sept. 25, 2017, 2:26 p.m.)


Review request for mesos and Kevin Klues.


Changes
-------

Resolved issues.


Summary (updated)
-----------------

Added CLI utility functions to verify addresses.


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


Repository: mesos


Description
-------

This will be used by future plugins.


Diffs (updated)
-----

  src/python/cli_new/lib/cli/util.py 7371f83543ed527bea8dbf2fe4e20d92ef8e4492 


Diff: https://reviews.apache.org/r/61212/diff/4/

Changes: https://reviews.apache.org/r/61212/diff/3-4/


Testing
-------

Tested manually, PEP8 and Pylint used to make sure that the code style is correct.


Thanks,

Armand Grillet


Re: Review Request 61212: Added CLI utility functions to check IPs and ports.

Posted by Armand Grillet <ag...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61212/
-----------------------------------------------------------

(Updated Sept. 4, 2017, 2:46 p.m.)


Review request for mesos and Kevin Klues.


Changes
-------

Updated functions to be more useful for the next commands.


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


Repository: mesos


Description
-------

This will be used by future plugins.


Diffs (updated)
-----

  src/python/cli_new/lib/cli/util.py 7371f83543ed527bea8dbf2fe4e20d92ef8e4492 


Diff: https://reviews.apache.org/r/61212/diff/3/

Changes: https://reviews.apache.org/r/61212/diff/2-3/


Testing
-------

Tested manually, PEP8 and Pylint used to make sure that the code style is correct.


Thanks,

Armand Grillet