You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Kevin Klues <kl...@gmail.com> on 2017/05/29 23:32:34 UTC
Review Request 59634: Updated 'config.py' in the new Mesos CLI to take
settings as parameter.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59634/
-----------------------------------------------------------
Review request for mesos and Armand Grillet.
Repository: mesos
Description
-------
Previously, we were importing settings directly from the `bin`
directory, which is obviously not portable. Instead, we need to be
passing the settings into the constructor for the config so that it
can access it from there.
Diffs
-----
src/cli_new/lib/cli/config.py 353785b8bdc13b745d2dc9469201c16a8816ebc6
Diff: https://reviews.apache.org/r/59634/diff/1/
Testing
-------
Tested manually, PEP8 and Pylint used to make sure that the code style is correct.
Thanks,
Kevin Klues
Re: Review Request 59634: Updated 'config.py' in the new Mesos CLI to
take settings as parameter.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59634/#review176338
-----------------------------------------------------------
Bad patch!
Reviews applied: [59634]
Failed command: python support/apply-reviews.py -n -r 59634
Error:
error: src/cli_new/lib/cli/config.py: does not exist in index
Full log: http://mesos-winbot.westus.cloudapp.azure.com/../logs/console
- Mesos Reviewbot Windows
On May 29, 2017, 11:32 p.m., Kevin Klues wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59634/
> -----------------------------------------------------------
>
> (Updated May 29, 2017, 11:32 p.m.)
>
>
> Review request for mesos and Armand Grillet.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Previously, we were importing settings directly from the `bin`
> directory, which is obviously not portable. Instead, we need to be
> passing the settings into the constructor for the config so that it
> can access it from there.
>
>
> Diffs
> -----
>
> src/cli_new/lib/cli/config.py 353785b8bdc13b745d2dc9469201c16a8816ebc6
>
>
> Diff: https://reviews.apache.org/r/59634/diff/1/
>
>
> Testing
> -------
>
> Tested manually, PEP8 and Pylint used to make sure that the code style is correct.
>
>
> Thanks,
>
> Kevin Klues
>
>
Re: Review Request 59634: Updated 'config.py' in the new Mesos CLI to
take settings as parameter.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59634/#review176445
-----------------------------------------------------------
Patch looks great!
Reviews applied: [59634]
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 May 30, 2017, 9:13 p.m., Kevin Klues wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59634/
> -----------------------------------------------------------
>
> (Updated May 30, 2017, 9:13 p.m.)
>
>
> Review request for mesos and Armand Grillet.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Previously, we were importing settings directly from the `bin`
> directory, which is obviously not portable. Instead, we need to be
> passing the settings into the constructor for the config so that it
> can access it from there.
>
>
> Diffs
> -----
>
> src/cli_new/bin/main.py d0ec2bdb04631fbaa62efb6ae67e138140a1296a
> src/cli_new/lib/cli/config.py 353785b8bdc13b745d2dc9469201c16a8816ebc6
>
>
> Diff: https://reviews.apache.org/r/59634/diff/2/
>
>
> Testing
> -------
>
> Tested manually, PEP8 and Pylint used to make sure that the code style is correct.
>
>
> Thanks,
>
> Kevin Klues
>
>
Re: Review Request 59634: Updated 'config.py' in the new Mesos CLI to
take settings as parameter.
Posted by Armand Grillet <ag...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59634/#review176428
-----------------------------------------------------------
Ship it!
The ReviewBot is acting strange, I have tested the patch and the CLI seems to work perfectly.
- Armand Grillet
On May 30, 2017, 9:13 p.m., Kevin Klues wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59634/
> -----------------------------------------------------------
>
> (Updated May 30, 2017, 9:13 p.m.)
>
>
> Review request for mesos and Armand Grillet.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Previously, we were importing settings directly from the `bin`
> directory, which is obviously not portable. Instead, we need to be
> passing the settings into the constructor for the config so that it
> can access it from there.
>
>
> Diffs
> -----
>
> src/cli_new/bin/main.py d0ec2bdb04631fbaa62efb6ae67e138140a1296a
> src/cli_new/lib/cli/config.py 353785b8bdc13b745d2dc9469201c16a8816ebc6
>
>
> Diff: https://reviews.apache.org/r/59634/diff/2/
>
>
> Testing
> -------
>
> Tested manually, PEP8 and Pylint used to make sure that the code style is correct.
>
>
> Thanks,
>
> Kevin Klues
>
>
Re: Review Request 59634: Updated 'config.py' in the new Mesos CLI to
take settings as parameter.
Posted by Kevin Klues <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59634/
-----------------------------------------------------------
(Updated June 12, 2017, 3:48 p.m.)
Review request for mesos and Armand Grillet.
Changes
-------
Updated the JIRA to point to the unit test infrastructure addition since this is a dependency of that.
Bugs: MESOS-7283
https://issues.apache.org/jira/browse/MESOS-7283
Repository: mesos
Description
-------
Previously, we were importing settings directly from the `bin`
directory, which is obviously not portable. Instead, we need to be
passing the settings into the constructor for the config so that it
can access it from there.
Diffs
-----
src/cli_new/bin/main.py d0ec2bdb04631fbaa62efb6ae67e138140a1296a
src/cli_new/lib/cli/config.py 353785b8bdc13b745d2dc9469201c16a8816ebc6
Diff: https://reviews.apache.org/r/59634/diff/2/
Testing
-------
Tested manually, PEP8 and Pylint used to make sure that the code style is correct.
Thanks,
Kevin Klues
Re: Review Request 59634: Updated 'config.py' in the new Mesos CLI to
take settings as parameter.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59634/#review176374
-----------------------------------------------------------
Bad patch!
Reviews applied: [59634]
Failed command: python support/apply-reviews.py -n -r 59634
Error:
error: patch failed: src/cli_new/bin/main.py:84
error: src/cli_new/bin/main.py: patch does not apply
error: src/cli_new/lib/cli/config.py: does not exist in index
Full log: http://mesos-winbot.westus.cloudapp.azure.com/../logs/console
- Mesos Reviewbot Windows
On May 30, 2017, 9:13 p.m., Kevin Klues wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59634/
> -----------------------------------------------------------
>
> (Updated May 30, 2017, 9:13 p.m.)
>
>
> Review request for mesos and Armand Grillet.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Previously, we were importing settings directly from the `bin`
> directory, which is obviously not portable. Instead, we need to be
> passing the settings into the constructor for the config so that it
> can access it from there.
>
>
> Diffs
> -----
>
> src/cli_new/bin/main.py d0ec2bdb04631fbaa62efb6ae67e138140a1296a
> src/cli_new/lib/cli/config.py 353785b8bdc13b745d2dc9469201c16a8816ebc6
>
>
> Diff: https://reviews.apache.org/r/59634/diff/2/
>
>
> Testing
> -------
>
> Tested manually, PEP8 and Pylint used to make sure that the code style is correct.
>
>
> Thanks,
>
> Kevin Klues
>
>
Re: Review Request 59634: Updated 'config.py' in the new Mesos CLI to
take settings as parameter.
Posted by Kevin Klues <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59634/
-----------------------------------------------------------
(Updated May 30, 2017, 9:13 p.m.)
Review request for mesos and Armand Grillet.
Changes
-------
Updated based on Armand's comments. I guess I just used it to run the follow on patch about unit testing and never tested `mesos` itself!
Repository: mesos
Description
-------
Previously, we were importing settings directly from the `bin`
directory, which is obviously not portable. Instead, we need to be
passing the settings into the constructor for the config so that it
can access it from there.
Diffs (updated)
-----
src/cli_new/bin/main.py d0ec2bdb04631fbaa62efb6ae67e138140a1296a
src/cli_new/lib/cli/config.py 353785b8bdc13b745d2dc9469201c16a8816ebc6
Diff: https://reviews.apache.org/r/59634/diff/2/
Changes: https://reviews.apache.org/r/59634/diff/1-2/
Testing
-------
Tested manually, PEP8 and Pylint used to make sure that the code style is correct.
Thanks,
Kevin Klues
Re: Review Request 59634: Updated 'config.py' in the new Mesos CLI to
take settings as parameter.
Posted by Armand Grillet <ag...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59634/#review176290
-----------------------------------------------------------
When using the command `mesos` after applying your patch on top of master, I got this error:
```
Traceback (most recent call last):
File "/Users/Armand/Code/apache-mesos/src/cli_new/bin/main.py", line 151, in <module>
main(sys.argv[1:])
File "/Users/Armand/Code/apache-mesos/src/cli_new/bin/main.py", line 87, in main
config = cli.config.Config()
TypeError: __init__() takes exactly 2 arguments (1 given)
```
Replacing the line 87 of bin/main.py by `config = cli.config.Config(settings)` solved the issue.
- Armand Grillet
On May 29, 2017, 11:32 p.m., Kevin Klues wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59634/
> -----------------------------------------------------------
>
> (Updated May 29, 2017, 11:32 p.m.)
>
>
> Review request for mesos and Armand Grillet.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Previously, we were importing settings directly from the `bin`
> directory, which is obviously not portable. Instead, we need to be
> passing the settings into the constructor for the config so that it
> can access it from there.
>
>
> Diffs
> -----
>
> src/cli_new/lib/cli/config.py 353785b8bdc13b745d2dc9469201c16a8816ebc6
>
>
> Diff: https://reviews.apache.org/r/59634/diff/1/
>
>
> Testing
> -------
>
> Tested manually, PEP8 and Pylint used to make sure that the code style is correct.
>
>
> Thanks,
>
> Kevin Klues
>
>
Re: Review Request 59634: Updated 'config.py' in the new Mesos CLI to
take settings as parameter.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59634/#review176289
-----------------------------------------------------------
Patch looks great!
Reviews applied: [59634]
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 May 29, 2017, 11:32 p.m., Kevin Klues wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59634/
> -----------------------------------------------------------
>
> (Updated May 29, 2017, 11:32 p.m.)
>
>
> Review request for mesos and Armand Grillet.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Previously, we were importing settings directly from the `bin`
> directory, which is obviously not portable. Instead, we need to be
> passing the settings into the constructor for the config so that it
> can access it from there.
>
>
> Diffs
> -----
>
> src/cli_new/lib/cli/config.py 353785b8bdc13b745d2dc9469201c16a8816ebc6
>
>
> Diff: https://reviews.apache.org/r/59634/diff/1/
>
>
> Testing
> -------
>
> Tested manually, PEP8 and Pylint used to make sure that the code style is correct.
>
>
> Thanks,
>
> Kevin Klues
>
>