Re: Review Request 57952: Added 'config' plugin to the new CLI.

[Kevin Klues]

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



As part of this patch , I expected `main.py` to be updated to explicitly load 
only the config plugin and then have the config plugin load the real set of 
plugins and pass them back to main. I.e. per our discussion on slack:

```
Maybe the right answer is to:
1) Move all TOML file parsing / validation out of settings.py and into the 
config plugin
2) Have main() explicitly instantiate the Config plugin by name
3) Replace the following call in `main()` from:
# Initialize the various plugins.
plugins = mesos.util.import_modules(settings.PLUGINS, "plugins")

to
plugins = ConfigPlugin(settings).plugins([])

3) The `ConfigPlugin.plugins()` command will then be in charge running 
`mesos.util.import_modules(plugins, "plugins")` over all plugins (both builtin 
and those found in the TOML file) to validate that they are importable and then 
return their loaded modules.
4) The main function would then process as before from there (edited)

[16:41] 
When `mesos config plugins` is invoked by the user, it can print out, e.g.:
NAMEDESCRIPTION
agent   Agent specific commands for the Mesos CLI
cluster Cluster specific commands for the Mesos CLI
container   Container specific commands for the Mesos CLI

which it can parse from the loaded modules from `PLUGIN_NAME` and `SHORT_HELP` 
(edited)

[16:44] 
or if there is an error with loading one of the plugins it can raise a 
CLIExpection() with a meaningful error message
```

- Kevin Klues


On April 12, 2017, 9:19 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57952/
> ---
> 
> (Updated April 12, 2017, 9:19 a.m.)
> 
> 
> Review request for mesos, Joseph Wu and Kevin Klues.
> 
> 
> Bugs: MESOS-7269
> https://issues.apache.org/jira/browse/MESOS-7269
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used to show the plugins available for the user.
> 
> 
> Diffs
> -
> 
>   src/cli_new/bin/settings.py 2f6162edc1722054bc44ad25956e6fe666d36c7f 
>   src/cli_new/lib/cli/plugins/config/__init__.py PRE-CREATION 
>   src/cli_new/lib/cli/plugins/config/main.py PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/57952/diff/6/
> 
> 
> Testing
> ---
> 
> Tested manually, PEP8 and Pylint used to make sure that the code style is 
> correct.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 57952: Added 'config' plugin to the new CLI.

[Joseph Wu]

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


Ship it!




LGTM.


src/cli_new/lib/cli/plugins/config/main.py
Lines 35 (patched)


How about:

"Interacts with the Mesos CLI configuration file"


- Joseph Wu


On April 12, 2017, 2:19 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57952/
> ---
> 
> (Updated April 12, 2017, 2:19 a.m.)
> 
> 
> Review request for mesos, Joseph Wu and Kevin Klues.
> 
> 
> Bugs: MESOS-7269
> https://issues.apache.org/jira/browse/MESOS-7269
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used to show the plugins available for the user.
> 
> 
> Diffs
> -
> 
>   src/cli_new/bin/settings.py 2f6162edc1722054bc44ad25956e6fe666d36c7f 
>   src/cli_new/lib/cli/plugins/config/__init__.py PRE-CREATION 
>   src/cli_new/lib/cli/plugins/config/main.py PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/57952/diff/6/
> 
> 
> Testing
> ---
> 
> Tested manually, PEP8 and Pylint used to make sure that the code style is 
> correct.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 57952: Added 'config' plugin to the new CLI.

[Armand Grillet]

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

(Updated April 12, 2017, 9:19 a.m.)


Review request for mesos, Joseph Wu and Kevin Klues.


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


Repository: mesos


Description (updated)
---

Used to show the plugins available for the user.


Diffs (updated)
-

  src/cli_new/bin/settings.py 2f6162edc1722054bc44ad25956e6fe666d36c7f 
  src/cli_new/lib/cli/plugins/config/__init__.py PRE-CREATION 
  src/cli_new/lib/cli/plugins/config/main.py PRE-CREATION 


Diff: https://reviews.apache.org/r/57952/diff/5/

Changes: https://reviews.apache.org/r/57952/diff/4-5/


Testing
---

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


Thanks,

Armand Grillet

Re: Review Request 57952: Added 'config' plugin to the new CLI.

[Joseph Wu]

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




src/cli_new/lib/cli/plugins/config/main.py
Lines 54-62 (patched)


Not sure if this `silent` field is really necessary.  It doesn't appear to 
be configurable (rather, you basically ignore the field in the plugin's 
methods)...



src/cli_new/lib/cli/util.py
Lines 155 (patched)


I'd like to the Table added in a separate commit, as it will probably be 
used in multiple plugins, in future.



src/cli_new/lib/cli/util.py
Lines 190 (patched)


s/no/not/



src/cli_new/lib/cli/util.py
Lines 192 (patched)


Period at the end...


- Joseph Wu


On April 11, 2017, 7:52 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57952/
> ---
> 
> (Updated April 11, 2017, 7:52 a.m.)
> 
> 
> Review request for mesos, Joseph Wu and Kevin Klues.
> 
> 
> Bugs: MESOS-7269
> https://issues.apache.org/jira/browse/MESOS-7269
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used to show the plugins available for the user.
> A Table abstraction has been added to print the plugins.
> 
> 
> Diffs
> -
> 
>   src/cli_new/bin/settings.py 2f6162edc1722054bc44ad25956e6fe666d36c7f 
>   src/cli_new/lib/cli/plugins/config/__init__.py PRE-CREATION 
>   src/cli_new/lib/cli/plugins/config/main.py PRE-CREATION 
>   src/cli_new/lib/cli/util.py ace07fb64e130e2f02d4ab5607ee1a84161ef88b 
> 
> 
> Diff: https://reviews.apache.org/r/57952/diff/4/
> 
> 
> Testing
> ---
> 
> Tested manually, PEP8 and Pylint used to make sure that the code style is 
> correct.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 57952: Added 'config' plugin to the new CLI.

[Armand Grillet]

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

(Updated April 11, 2017, 2:52 p.m.)


Review request for mesos, Joseph Wu and Kevin Klues.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Used to show the plugins available for the user.
A Table abstraction has been added to print the plugins.


Diffs (updated)
-

  src/cli_new/bin/settings.py 2f6162edc1722054bc44ad25956e6fe666d36c7f 
  src/cli_new/lib/cli/plugins/config/__init__.py PRE-CREATION 
  src/cli_new/lib/cli/plugins/config/main.py PRE-CREATION 
  src/cli_new/lib/cli/util.py ace07fb64e130e2f02d4ab5607ee1a84161ef88b 


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

Changes: https://reviews.apache.org/r/57952/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 57952: Added 'config' plugin to the new CLI.

[Armand Grillet]

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

(Updated April 10, 2017, 6:37 a.m.)


Review request for mesos, Joseph Wu and Kevin Klues.


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


Repository: mesos


Description (updated)
---

Used to show the plugins available for the user.
A Table abstraction has been added to print the plugins.


Diffs (updated)
-

  src/cli_new/bin/settings.py PRE-CREATION 
  src/cli_new/lib/mesos/plugins/config/__init__.py PRE-CREATION 
  src/cli_new/lib/mesos/plugins/config/main.py PRE-CREATION 
  src/cli_new/lib/mesos/util.py 87d2a65e78f04209566c1434b489b941d570ee01 


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

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


Testing
---

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


Thanks,

Armand Grillet

Re: Review Request 57952: Added 'config' plugin to the new CLI.

[Joseph Wu]

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




src/cli_new/lib/mesos/plugins/config/main.py
Lines 77-90 (patched)


I feel that the loading of plugins should still be done inside the 
`main.py` file, as it was before.  This would let us do all the loading in one 
location, versus potentially duplicating the plugin loading logic into 
`main.py` and this file.

We can still keep the config plugin, but limit its scope to reading, 
validating, and possibly writing the config file.


- Joseph Wu


On March 27, 2017, 5 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57952/
> ---
> 
> (Updated March 27, 2017, 5 a.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7269
> https://issues.apache.org/jira/browse/MESOS-7269
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used to load and show the plugins available for the user.
> 
> 
> Diffs
> -
> 
>   src/cli_new/bin/main.py bbfb52c894540158c70e0f50ebb8a277b692d54d 
>   src/cli_new/bin/settings.py PRE-CREATION 
>   src/cli_new/lib/mesos/plugins/config/__init__.py PRE-CREATION 
>   src/cli_new/lib/mesos/plugins/config/main.py PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/57952/diff/2/
> 
> 
> Testing
> ---
> 
> Tested manually, PEP8 and Pylint used to make sure that the code style is 
> correct.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 57952: Added 'config' plugin to the new CLI.

[Joseph Wu]

> On April 5, 2017, 5:10 p.m., Kevin Klues wrote:
> > src/cli_new/lib/mesos/plugins/config/main.py
> > Lines 96-102 (patched)
> > 
> >
> > I would actually leverage the Table abstraction to print th eplugins 
> > here. I think this might be a discarded RR from me that should probably be 
> > revived for this.

Armand, in case you want to revive the Table abstraction, here it is: 
https://reviews.apache.org/r/52948/


- Joseph


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


On March 27, 2017, 5 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57952/
> ---
> 
> (Updated March 27, 2017, 5 a.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7269
> https://issues.apache.org/jira/browse/MESOS-7269
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used to load and show the plugins available for the user.
> 
> 
> Diffs
> -
> 
>   src/cli_new/bin/main.py bbfb52c894540158c70e0f50ebb8a277b692d54d 
>   src/cli_new/bin/settings.py PRE-CREATION 
>   src/cli_new/lib/mesos/plugins/config/__init__.py PRE-CREATION 
>   src/cli_new/lib/mesos/plugins/config/main.py PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/57952/diff/2/
> 
> 
> Testing
> ---
> 
> Tested manually, PEP8 and Pylint used to make sure that the code style is 
> correct.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 57952: Added 'config' plugin to the new CLI.

[Kevin Klues]

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




src/cli_new/lib/mesos/plugins/config/main.py
Lines 96-102 (patched)


I would actually leverage the Table abstraction to print th eplugins here. 
I think this might be a discarded RR from me that should probably be revived 
for this.


- Kevin Klues


On March 27, 2017, noon, Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57952/
> ---
> 
> (Updated March 27, 2017, noon)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7269
> https://issues.apache.org/jira/browse/MESOS-7269
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used to load and show the plugins available for the user.
> 
> 
> Diffs
> -
> 
>   src/cli_new/bin/main.py bbfb52c894540158c70e0f50ebb8a277b692d54d 
>   src/cli_new/bin/settings.py PRE-CREATION 
>   src/cli_new/lib/mesos/plugins/config/__init__.py PRE-CREATION 
>   src/cli_new/lib/mesos/plugins/config/main.py PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/57952/diff/2/
> 
> 
> Testing
> ---
> 
> Tested manually, PEP8 and Pylint used to make sure that the code style is 
> correct.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 57952: Added 'config' plugin to the new CLI.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [57896, 57951, 57952]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On March 27, 2017, noon, Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57952/
> ---
> 
> (Updated March 27, 2017, noon)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7269
> https://issues.apache.org/jira/browse/MESOS-7269
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used to load and show the plugins available for the user.
> 
> 
> Diffs
> -
> 
>   src/cli_new/bin/main.py bbfb52c894540158c70e0f50ebb8a277b692d54d 
>   src/cli_new/bin/settings.py PRE-CREATION 
>   src/cli_new/lib/mesos/plugins/config/__init__.py PRE-CREATION 
>   src/cli_new/lib/mesos/plugins/config/main.py PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/57952/diff/2/
> 
> 
> Testing
> ---
> 
> Tested manually, PEP8 and Pylint used to make sure that the code style is 
> correct.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 57952: Added 'config' plugin to the new CLI.

[Kevin Klues]

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



I think we should strip everything out of this commit except the parsing of the 
plugins array from the TOML file (which isn't in this commit at all yet). We 
should move that functionality out of settings.py and into here.

- Kevin Klues


On March 27, 2017, noon, Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57952/
> ---
> 
> (Updated March 27, 2017, noon)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7269
> https://issues.apache.org/jira/browse/MESOS-7269
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used to show and validate the configuration file given by the user.
> 
> 
> Diffs
> -
> 
>   src/cli_new/bin/settings.py PRE-CREATION 
>   src/cli_new/lib/mesos/plugins/config/__init__.py PRE-CREATION 
>   src/cli_new/lib/mesos/plugins/config/main.py PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/57952/diff/1/
> 
> 
> Testing
> ---
> 
> Tested manually, PEP8 and Pylint used to make sure that the code style is 
> correct.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 57952: Added 'config' plugin to the new CLI.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [57896, 57951, 57952]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On March 27, 2017, 5 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57952/
> ---
> 
> (Updated March 27, 2017, 5 a.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7269
> https://issues.apache.org/jira/browse/MESOS-7269
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used to show and validate the configuration file given by the user.
> 
> 
> Diffs
> -
> 
>   src/cli_new/bin/settings.py PRE-CREATION 
>   src/cli_new/lib/mesos/plugins/config/__init__.py PRE-CREATION 
>   src/cli_new/lib/mesos/plugins/config/main.py PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/57952/diff/1/
> 
> 
> Testing
> ---
> 
> Tested manually, PEP8 and Pylint used to make sure that the code style is 
> correct.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 57952: Added 'config' plugin to the new CLI.

[Armand Grillet]

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

(Updated March 27, 2017, noon)


Review request for mesos and Kevin Klues.


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


Repository: mesos


Description
---

Used to show and validate the configuration file given by the user.


Diffs
-

  src/cli_new/bin/settings.py PRE-CREATION 
  src/cli_new/lib/mesos/plugins/config/__init__.py PRE-CREATION 
  src/cli_new/lib/mesos/plugins/config/main.py PRE-CREATION 


Diff: https://reviews.apache.org/r/57952/diff/1/


Testing
---

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


Thanks,

Armand Grillet