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/25 14:03:12 UTC
Re: Review Request 60088: CLI: Added 'master' key as an acceptable
key in config.toml.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60088/
-----------------------------------------------------------
(Updated Sept. 25, 2017, 2:03 p.m.)
Review request for mesos and Kevin Klues.
Changes
-------
Updated `master()` function.
Summary (updated)
-----------------
CLI: Added 'master' key as an acceptable key in config.toml.
Bugs: MESOS-7284
https://issues.apache.org/jira/browse/MESOS-7284
Repository: mesos
Description (updated)
-------
This key is a field that has to be composed of
an `address` or `zookeeper` field, but not both.
Diffs (updated)
-----
src/python/cli_new/README.md c5475c7f4632fde5303ed1c901918d9088eac660
src/python/cli_new/lib/cli/config.py 36a32f94bb12a033705b20f3c91d8bba972ba6c5
src/python/cli_new/lib/cli/util.py 7371f83543ed527bea8dbf2fe4e20d92ef8e4492
Diff: https://reviews.apache.org/r/60088/diff/6/
Changes: https://reviews.apache.org/r/60088/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 60088: CLI: Added 'master' key as an acceptable
key in config.toml.
Posted by Kevin Klues <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60088/#review186253
-----------------------------------------------------------
Ship it!
Ship It!
- Kevin Klues
On Sept. 25, 2017, 2:03 p.m., Armand Grillet wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60088/
> -----------------------------------------------------------
>
> (Updated Sept. 25, 2017, 2:03 p.m.)
>
>
> Review request for mesos and Kevin Klues.
>
>
> Bugs: MESOS-7284
> https://issues.apache.org/jira/browse/MESOS-7284
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This key is a field that has to be composed of
> an `address` or `zookeeper` field, but not both.
>
>
> Diffs
> -----
>
> src/python/cli_new/README.md c5475c7f4632fde5303ed1c901918d9088eac660
> src/python/cli_new/lib/cli/config.py 36a32f94bb12a033705b20f3c91d8bba972ba6c5
> src/python/cli_new/lib/cli/util.py 7371f83543ed527bea8dbf2fe4e20d92ef8e4492
>
>
> Diff: https://reviews.apache.org/r/60088/diff/7/
>
>
> Testing
> -------
>
> Tested manually, PEP8 and Pylint used to make sure that the code style is correct.
>
>
> Thanks,
>
> Armand Grillet
>
>
Re: Review Request 60088: CLI: Added 'master' key as an acceptable
key in config.toml.
Posted by Kevin Klues <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60088/#review186245
-----------------------------------------------------------
src/python/cli_new/README.md
Lines 82 (patched)
<https://reviews.apache.org/r/60088/#comment262723>
I would add a "For example:" here.
src/python/cli_new/lib/cli/config.py
Lines 54 (patched)
<https://reviews.apache.org/r/60088/#comment262724>
We shouldn't need this anymore.
src/python/cli_new/lib/cli/config.py
Lines 58 (patched)
<https://reviews.apache.org/r/60088/#comment262725>
We should probably pull this in from the constants file instead of hard coding it as a string here.
src/python/cli_new/lib/cli/config.py
Lines 65 (patched)
<https://reviews.apache.org/r/60088/#comment262726>
The indentation here seems wrong.
src/python/cli_new/lib/cli/config.py
Lines 67 (patched)
<https://reviews.apache.org/r/60088/#comment262728>
One too many spaces here
src/python/cli_new/lib/cli/config.py
Lines 70 (patched)
<https://reviews.apache.org/r/60088/#comment262727>
the indentation here seems wrong
src/python/cli_new/lib/cli/config.py
Lines 80 (patched)
<https://reviews.apache.org/r/60088/#comment262733>
The 'master' address
src/python/cli_new/lib/cli/config.py
Lines 88 (patched)
<https://reviews.apache.org/r/60088/#comment262729>
We need to first check if 'addresses' in 'zk_field' before we can index it.
src/python/cli_new/lib/cli/config.py
Lines 91 (patched)
<https://reviews.apache.org/r/60088/#comment262730>
We need to first check if 'path' in 'zk_field' before we can index it.
src/python/cli_new/lib/cli/config.py
Lines 95 (patched)
<https://reviews.apache.org/r/60088/#comment262731>
I think it's OK to start with a `/` it just can't be *only* `/'
src/python/cli_new/lib/cli/config.py
Lines 102 (patched)
<https://reviews.apache.org/r/60088/#comment262732>
The 'zookeeper' address.
src/python/cli_new/lib/cli/config.py
Lines 111 (patched)
<https://reviews.apache.org/r/60088/#comment262734>
s/leader/leading/
- Kevin Klues
On Sept. 25, 2017, 2:03 p.m., Armand Grillet wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60088/
> -----------------------------------------------------------
>
> (Updated Sept. 25, 2017, 2:03 p.m.)
>
>
> Review request for mesos and Kevin Klues.
>
>
> Bugs: MESOS-7284
> https://issues.apache.org/jira/browse/MESOS-7284
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This key is a field that has to be composed of
> an `address` or `zookeeper` field, but not both.
>
>
> Diffs
> -----
>
> src/python/cli_new/README.md c5475c7f4632fde5303ed1c901918d9088eac660
> src/python/cli_new/lib/cli/config.py 36a32f94bb12a033705b20f3c91d8bba972ba6c5
> src/python/cli_new/lib/cli/util.py 7371f83543ed527bea8dbf2fe4e20d92ef8e4492
>
>
> Diff: https://reviews.apache.org/r/60088/diff/6/
>
>
> Testing
> -------
>
> Tested manually, PEP8 and Pylint used to make sure that the code style is correct.
>
>
> Thanks,
>
> Armand Grillet
>
>
Re: Review Request 60088: CLI: Added 'master' key as an acceptable
key in config.toml.
Posted by Kevin Klues <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60088/#review186252
-----------------------------------------------------------
src/python/cli_new/lib/cli/config.py
Lines 57 (patched)
<https://reviews.apache.org/r/60088/#comment262737>
I'll remove this comment before committing since we now pull i nthe constants from elsewhere.
- Kevin Klues
On Sept. 25, 2017, 2:03 p.m., Armand Grillet wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60088/
> -----------------------------------------------------------
>
> (Updated Sept. 25, 2017, 2:03 p.m.)
>
>
> Review request for mesos and Kevin Klues.
>
>
> Bugs: MESOS-7284
> https://issues.apache.org/jira/browse/MESOS-7284
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This key is a field that has to be composed of
> an `address` or `zookeeper` field, but not both.
>
>
> Diffs
> -----
>
> src/python/cli_new/README.md c5475c7f4632fde5303ed1c901918d9088eac660
> src/python/cli_new/lib/cli/config.py 36a32f94bb12a033705b20f3c91d8bba972ba6c5
> src/python/cli_new/lib/cli/util.py 7371f83543ed527bea8dbf2fe4e20d92ef8e4492
>
>
> Diff: https://reviews.apache.org/r/60088/diff/7/
>
>
> Testing
> -------
>
> Tested manually, PEP8 and Pylint used to make sure that the code style is correct.
>
>
> Thanks,
>
> Armand Grillet
>
>
Re: Review Request 60088: CLI: Added 'master' key as an acceptable
key in config.toml.
Posted by Kevin Klues <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60088/#review185986
-----------------------------------------------------------
src/python/cli_new/README.md
Lines 82 (patched)
<https://reviews.apache.org/r/60088/#comment262334>
At the end of this comment I'd add "For example:"
src/python/cli_new/lib/cli/config.py
Lines 72 (patched)
<https://reviews.apache.org/r/60088/#comment262333>
Let's verify the address format here and throw an error if it's wrong.
- Kevin Klues
On Sept. 25, 2017, 2:03 p.m., Armand Grillet wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60088/
> -----------------------------------------------------------
>
> (Updated Sept. 25, 2017, 2:03 p.m.)
>
>
> Review request for mesos and Kevin Klues.
>
>
> Bugs: MESOS-7284
> https://issues.apache.org/jira/browse/MESOS-7284
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This key is a field that has to be composed of
> an `address` or `zookeeper` field, but not both.
>
>
> Diffs
> -----
>
> src/python/cli_new/README.md c5475c7f4632fde5303ed1c901918d9088eac660
> src/python/cli_new/lib/cli/config.py 36a32f94bb12a033705b20f3c91d8bba972ba6c5
> src/python/cli_new/lib/cli/util.py 7371f83543ed527bea8dbf2fe4e20d92ef8e4492
>
>
> Diff: https://reviews.apache.org/r/60088/diff/6/
>
>
> Testing
> -------
>
> Tested manually, PEP8 and Pylint used to make sure that the code style is correct.
>
>
> Thanks,
>
> Armand Grillet
>
>