Re: Review Request 64970: Replace ad hoc venv under support/ with tox.

2018-03-01 Thread Armand Grillet 

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



There is still one issue with this patch, I have seen some verbose logs that 
are repeated at each run:

```
apache-mesos (tox) $ git commit -m "Test."
No C++ files to lint
No JavaScript files to lint
Checking 1 Python file
py27-lint installed: 
astroid==1.4.8,attrs==17.4.0,backports.functools-lru-cache==1.2.1,configparser==3.5.0,coverage==4.5.1,docopt==0.6.2,funcsigs==1.0.2,isort==4.2.5,kazoo==2.3.1,lazy-object-proxy==1.2.2,mccabe==0.5.2,mock==2.0.0,parse==1.8.0,pbr==3.1.1,pluggy==0.6.0,py==1.5.2,Pygments==2.1.3,PyInstaller==3.1.1,pylint==1.6.4,pytest==3.4.1,pytest-cov==2.5.1,six==1.10.0,termcolor==1.1.0,toml==0.9.2,tox==2.7.0,virtualenv==15.1.0,wrapt==1.10.8
py27-lint runtests: PYTHONHASHSEED='1333548182'
py27-lint runtests: commands[0] | pylint 
--rcfile=../../../support/pylint.config 
--rcfile=/Users/Armand/Code/apache-mesos/support/pylint.config 
lib/cli/plugins/config/main.py
* Module cli.plugins.config.main
W: 80,19: Unused argument 'argv' (unused-argument)
ERROR: InvocationError: 
'/Users/Armand/Code/apache-mesos/src/python/cli_new/.tox/py27-lint/bin/pylint 
--rcfile=../../../support/pylint.config 
--rcfile=/Users/Armand/Code/apache-mesos/support/pylint.config 
lib/cli/plugins/config/main.py'
___ summary 
ERROR:   py27-lint: commands failed
Total errors found: 1
apache-mesos (tox) $ vi src/python/cli_new/lib/cli/plugins/config/main.py
apache-mesos (tox) $ git add src/python/cli_new/lib/cli/plugins/config/main.py
apache-mesos (tox) $ git commit -m "Test."
No C++ files to lint
No JavaScript files to lint
Checking 1 Python file
py27-lint installed: 
astroid==1.4.8,attrs==17.4.0,backports.functools-lru-cache==1.2.1,configparser==3.5.0,coverage==4.5.1,docopt==0.6.2,funcsigs==1.0.2,isort==4.2.5,kazoo==2.3.1,lazy-object-proxy==1.2.2,mccabe==0.5.2,mock==2.0.0,parse==1.8.0,pbr==3.1.1,pluggy==0.6.0,py==1.5.2,Pygments==2.1.3,PyInstaller==3.1.1,pylint==1.6.4,pytest==3.4.1,pytest-cov==2.5.1,six==1.10.0,termcolor==1.1.0,toml==0.9.2,tox==2.7.0,virtualenv==15.1.0,wrapt==1.10.8
py27-lint runtests: PYTHONHASHSEED='87451174'
py27-lint runtests: commands[0] | pylint 
--rcfile=../../../support/pylint.config 
--rcfile=/Users/Armand/Code/apache-mesos/support/pylint.config 
lib/cli/plugins/config/main.py
___ summary 
  py27-lint: commands succeeded
  congratulations :)
Total errors found: 0
[tox 46cc7db60] Test.
 1 file changed, 1 insertion(+), 1 deletion(-)
```
I'm talking about the lines starting with `py27-lint installed:`. Can we 
improve that by not reinstalling `py27-lint` if it is already installed or just 
reducing the verbosity of the logs to hide the installation?


support/mesos-style.py
Lines 376 (patched)


s/`Activate`/`Activates` and s/`run`/`runs`.



support/mesos-style.py
Lines 401 (patched)


Not sure if adding this method is necessary (specially in that patch) but 
it does not hurt.


- Armand Grillet


On Feb. 23, 2018, 7:58 a.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64970/
> ---
> 
> (Updated Feb. 23, 2018, 7:58 a.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use tox for linting and testing code living uder src/python.
> 
> At the moment, all linting is done through the same `pylint`
> installation under support/.virtualenv, which requires ALL dependencies
> (i.e. pip-requirements.txt, requirements.in scattered in various
> directories) to be installed in the same virtualenv, making things
> really messy -- e.g. when I've changed some code under `src/python/lib`,
> but don't have the dev virtualenv activated, linting will fail since
> none of the dependencies under `src/python/lib` have been installed.
> 
> Using tox, we can solve this problem by distributing a "test spec"
> (tox.ini) in each of the python source directories which are aware of
> its local dependencies only. To test or lint the code there would be as
> simple as running `tox -e py27-lint `, and the corresponding
> virtualenv and test dependencies would automatically be setup.
> 
> This patch modifies `support/mesos-style.py` to install `tox` in
> `support/.virtualenv` and delegates linting to a `tox` call when it sees
> python directories that have tox setup for it. Linting for all other
> languages will not be effected.
> 
> Testing Done:
> 1. intentionally create

Re: Review Request 64970: Replace ad hoc venv under support/ with tox.

2018-02-23 Thread Mesos Reviewbot 

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



Patch looks great!

Reviews applied: [64970]

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

- Mesos Reviewbot


On Feb. 23, 2018, 7:58 a.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64970/
> ---
> 
> (Updated Feb. 23, 2018, 7:58 a.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use tox for linting and testing code living uder src/python.
> 
> At the moment, all linting is done through the same `pylint`
> installation under support/.virtualenv, which requires ALL dependencies
> (i.e. pip-requirements.txt, requirements.in scattered in various
> directories) to be installed in the same virtualenv, making things
> really messy -- e.g. when I've changed some code under `src/python/lib`,
> but don't have the dev virtualenv activated, linting will fail since
> none of the dependencies under `src/python/lib` have been installed.
> 
> Using tox, we can solve this problem by distributing a "test spec"
> (tox.ini) in each of the python source directories which are aware of
> its local dependencies only. To test or lint the code there would be as
> simple as running `tox -e py27-lint `, and the corresponding
> virtualenv and test dependencies would automatically be setup.
> 
> This patch modifies `support/mesos-style.py` to install `tox` in
> `support/.virtualenv` and delegates linting to a `tox` call when it sees
> python directories that have tox setup for it. Linting for all other
> languages will not be effected.
> 
> Testing Done:
> 1. intentionally create a lint error, such as extra spaces before a
> parens in a python file
> 2. run the pre-commit hook and see tox in action
> 
> Reviewed at https://reviews.apache.org/r/64970/
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/tox.ini PRE-CREATION 
>   src/python/lib/requirements-test.in 
> b2b73aab65377d9310797203ea84c5150ae60805 
>   src/python/lib/tox.ini fd5e89c77c8608fea21e9caad814c6e111ad57db 
>   support/mesos-style.py 47ec36949010fa511d1b3974739c5ad5c03f6f7f 
> 
> 
> Diff: https://reviews.apache.org/r/64970/diff/4/
> 
> 
> Testing
> ---
> 
> 1. intentionally create a lint error, such as extra spaces before a parens in 
> a python file
> 2. run the pre-commit hook and see tox in action
> 
> 
> Thanks,
> 
> Eric Chung
> 
>

Re: Review Request 64970: Replace ad hoc venv under support/ with tox.

2018-02-23 Thread Mesos Reviewbot Windows 

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



PASS: Mesos patch 64970 was successfully built and tested.

Reviews applied: `['64970']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64970

- Mesos Reviewbot Windows


On Feb. 23, 2018, 7:58 a.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64970/
> ---
> 
> (Updated Feb. 23, 2018, 7:58 a.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use tox for linting and testing code living uder src/python.
> 
> At the moment, all linting is done through the same `pylint`
> installation under support/.virtualenv, which requires ALL dependencies
> (i.e. pip-requirements.txt, requirements.in scattered in various
> directories) to be installed in the same virtualenv, making things
> really messy -- e.g. when I've changed some code under `src/python/lib`,
> but don't have the dev virtualenv activated, linting will fail since
> none of the dependencies under `src/python/lib` have been installed.
> 
> Using tox, we can solve this problem by distributing a "test spec"
> (tox.ini) in each of the python source directories which are aware of
> its local dependencies only. To test or lint the code there would be as
> simple as running `tox -e py27-lint `, and the corresponding
> virtualenv and test dependencies would automatically be setup.
> 
> This patch modifies `support/mesos-style.py` to install `tox` in
> `support/.virtualenv` and delegates linting to a `tox` call when it sees
> python directories that have tox setup for it. Linting for all other
> languages will not be effected.
> 
> Testing Done:
> 1. intentionally create a lint error, such as extra spaces before a
> parens in a python file
> 2. run the pre-commit hook and see tox in action
> 
> Reviewed at https://reviews.apache.org/r/64970/
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/tox.ini PRE-CREATION 
>   src/python/lib/requirements-test.in 
> b2b73aab65377d9310797203ea84c5150ae60805 
>   src/python/lib/tox.ini fd5e89c77c8608fea21e9caad814c6e111ad57db 
>   support/mesos-style.py 47ec36949010fa511d1b3974739c5ad5c03f6f7f 
> 
> 
> Diff: https://reviews.apache.org/r/64970/diff/4/
> 
> 
> Testing
> ---
> 
> 1. intentionally create a lint error, such as extra spaces before a parens in 
> a python file
> 2. run the pre-commit hook and see tox in action
> 
> 
> Thanks,
> 
> Eric Chung
> 
>

Re: Review Request 64970: Replace ad hoc venv under support/ with tox.

2018-02-22 Thread Eric Chung 

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

(Updated Feb. 23, 2018, 7:58 a.m.)


Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.


Summary (updated)
-

Replace ad hoc venv under support/ with tox.


Repository: mesos


Description
---

Use tox for linting and testing code living uder src/python.

At the moment, all linting is done through the same `pylint`
installation under support/.virtualenv, which requires ALL dependencies
(i.e. pip-requirements.txt, requirements.in scattered in various
directories) to be installed in the same virtualenv, making things
really messy -- e.g. when I've changed some code under `src/python/lib`,
but don't have the dev virtualenv activated, linting will fail since
none of the dependencies under `src/python/lib` have been installed.

Using tox, we can solve this problem by distributing a "test spec"
(tox.ini) in each of the python source directories which are aware of
its local dependencies only. To test or lint the code there would be as
simple as running `tox -e py27-lint `, and the corresponding
virtualenv and test dependencies would automatically be setup.

This patch modifies `support/mesos-style.py` to install `tox` in
`support/.virtualenv` and delegates linting to a `tox` call when it sees
python directories that have tox setup for it. Linting for all other
languages will not be effected.

Testing Done:
1. intentionally create a lint error, such as extra spaces before a
parens in a python file
2. run the pre-commit hook and see tox in action

Reviewed at https://reviews.apache.org/r/64970/


Diffs (updated)
-

  src/python/cli_new/tox.ini PRE-CREATION 
  src/python/lib/requirements-test.in b2b73aab65377d9310797203ea84c5150ae60805 
  src/python/lib/tox.ini fd5e89c77c8608fea21e9caad814c6e111ad57db 
  support/mesos-style.py 47ec36949010fa511d1b3974739c5ad5c03f6f7f 


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

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


Testing
---

1. intentionally create a lint error, such as extra spaces before a parens in a 
python file
2. run the pre-commit hook and see tox in action


Thanks,

Eric Chung

Re: Review Request 64970: Replace ad hoc venv under support/ with tox.

2018-01-18 Thread Eric Chung 

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

(Updated Jan. 18, 2018, 11:46 p.m.)


Review request for mesos, Armand Grillet and Kevin Klues.


Repository: mesos


Description (updated)
---

Use tox for linting and testing code living uder src/python.

At the moment, all linting is done through the same `pylint`
installation under support/.virtualenv, which requires ALL dependencies
(i.e. pip-requirements.txt, requirements.in scattered in various
directories) to be installed in the same virtualenv, making things
really messy -- e.g. when I've changed some code under `src/python/lib`,
but don't have the dev virtualenv activated, linting will fail since
none of the dependencies under `src/python/lib` have been installed.

Using tox, we can solve this problem by distributing a "test spec"
(tox.ini) in each of the python source directories which are aware of
its local dependencies only. To test or lint the code there would be as
simple as running `tox -e py27-lint `, and the corresponding
virtualenv and test dependencies would automatically be setup.

This patch modifies `support/mesos-style.py` to install `tox` in
`support/.virtualenv` and delegates linting to a `tox` call when it sees
python directories that have tox setup for it. Linting for all other
languages will not be effected.

Testing Done:
1. intentionally create a lint error, such as extra spaces before a
parens in a python file
2. run the pre-commit hook and see tox in action

Reviewed at https://reviews.apache.org/r/64970/


Diffs (updated)
-

  src/python/cli_new/tox.ini PRE-CREATION 
  src/python/lib/requirements-test.in b2b73aab65377d9310797203ea84c5150ae60805 
  src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
  support/mesos-style.py 1b34ea2d9afa8f17b545841cea7a6853a6e18144 


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

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


Testing
---

1. intentionally create a lint error, such as extra spaces before a parens in a 
python file
2. run the pre-commit hook and see tox in action


Thanks,

Eric Chung

Re: Review Request 64970: Replace ad hoc venv under support/ with tox.

2018-01-18 Thread Eric Chung 

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

(Updated Jan. 18, 2018, 11:20 p.m.)


Review request for mesos, Armand Grillet and Kevin Klues.


Repository: mesos


Description (updated)
---

Use tox for linting and testing code living uder src/python.

At the moment, all linting is done through the same `pylint` installation under 
support/.virtualenv, which requires ALL dependencies (i.e. 
pip-requirements.txt, requirements.in scattered in various directories) to be 
installed in the same virtualenv, making things really messy -- e.g. when I've 
changed some code under `src/python/lib`, but don't have the dev virtualenv 
activated, linting will fail since none of the dependencies under 
`src/python/lib` have been installed.

Using tox, we can solve this problem by distributing a "test spec" (tox.ini) in 
each of the python source directories which are aware of its local dependencies 
only. To test or lint the code there would be as simple as running `tox -e 
py27-lint `, and the corresponding virtualenv and test dependencies 
would automatically be setup.

This patch modifies `support/mesos-style.py` to install `tox` in 
`support/.virtualenv` and delegates linting to a `tox` call when it sees python 
directories that have tox setup for it. Linting for all other languages will 
not be effected.


Diffs (updated)
-

  src/python/cli_new/tox.ini PRE-CREATION 
  src/python/lib/requirements-test.in b2b73aab65377d9310797203ea84c5150ae60805 
  src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
  support/mesos-style.py 1b34ea2d9afa8f17b545841cea7a6853a6e18144 


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

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


Testing
---

1. intentionally create a lint error, such as extra spaces before a parens in a 
python file
2. run the pre-commit hook and see tox in action


Thanks,

Eric Chung

Re: Review Request 64970: Replace ad hoc venv under support/ with tox.

2018-01-05 Thread Armand Grillet 

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



I agree with Kevin, hhaving tox managing our virtualenvs rather than doing it 
manually is a good idea but the cost of having a new tool that Mesos 
contributors need to install for the Python linting process seems too high. 
This is reinforced by the fact that the JS linter will still require a manually 
managed virtualenv in `/support`, this patch will thus add a new virtualenv 
instead of replacing/removing an existent one.


support/mesos-style.py
Line 150 (original), 150 (patched)


`mesos/support/tox.ini` does not exist and is not added in this patch, is 
that normal?


- Armand Grillet


On Jan. 5, 2018, 8:26 a.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64970/
> ---
> 
> (Updated Jan. 5, 2018, 8:26 a.m.)
> 
> 
> Review request for mesos, Armand Grillet and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replace ad hoc venv under support/ with tox.
> 
> This patch changes the ad hoc .virtualenv created under support/ with tox, a 
> tool that manages virtualenvs with a declaritive config file. The cool thing 
> about it is that you can actually have multiple tox.ini files distributed in 
> different python source trees, so you don't really have to install all of the 
> depedencies into a single virtualenv; all you need to do is to run `tox -e 
>  ` at the root of the source tree.
> 
> 
> Diffs
> -
> 
>   support/.gitignore a21a0f95b9113eae2881d2e346821c86761bb2bc 
>   support/mesos-style.py 1b34ea2d9afa8f17b545841cea7a6853a6e18144 
> 
> 
> Diff: https://reviews.apache.org/r/64970/diff/1/
> 
> 
> Testing
> ---
> 
> 1. intentionally create a lint error, such as extra spaces before a parens in 
> a python file
> 2. run the pre-commit hook and see tox in action
> 
> 
> Thanks,
> 
> Eric Chung
> 
>

Re: Review Request 64970: Replace ad hoc venv under support/ with tox.

2018-01-05 Thread Kevin Klues 

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



Is the assumption that the user already have `tox` installed on their box? I've 
never installed tox on my machine outside of a virtualenv (and would prefer not 
to have to unless absolutely necessary).

- Kevin Klues


On Jan. 5, 2018, 8:26 a.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64970/
> ---
> 
> (Updated Jan. 5, 2018, 8:26 a.m.)
> 
> 
> Review request for mesos, Armand Grillet and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replace ad hoc venv under support/ with tox.
> 
> This patch changes the ad hoc .virtualenv created under support/ with tox, a 
> tool that manages virtualenvs with a declaritive config file. The cool thing 
> about it is that you can actually have multiple tox.ini files distributed in 
> different python source trees, so you don't really have to install all of the 
> depedencies into a single virtualenv; all you need to do is to run `tox -e 
>  ` at the root of the source tree.
> 
> 
> Diffs
> -
> 
>   support/.gitignore a21a0f95b9113eae2881d2e346821c86761bb2bc 
>   support/mesos-style.py 1b34ea2d9afa8f17b545841cea7a6853a6e18144 
> 
> 
> Diff: https://reviews.apache.org/r/64970/diff/1/
> 
> 
> Testing
> ---
> 
> 1. intentionally create a lint error, such as extra spaces before a parens in 
> a python file
> 2. run the pre-commit hook and see tox in action
> 
> 
> Thanks,
> 
> Eric Chung
> 
>