Re: Review Request 64970: Use tox for linting and testing code living uder src/python.

2018-05-09 Thread Kevin Klues 

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




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


I am about to commit this, but I wanted to note some changes for posterity.

I am adding a bump to tox 3.0.0 as a precommit to this RR so that we can 
additionally pass the '-qq' to tox to suppress its output. I am also modifying 
this RR to pass this option here.

We want to see the output of the commands it issues, but we don't want to 
see the output of tox itself. The '-qq' option gives us this ability.


- Kevin Klues


On March 14, 2018, 3:05 p.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64970/
> ---
> 
> (Updated March 14, 2018, 3:05 p.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/tests/__init__.py PRE-CREATION 
>   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/6/
> 
> 
> 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: Use tox for linting and testing code living uder src/python.

2018-04-12 Thread Eric Chung 


> On April 6, 2018, 9:46 a.m., Armand Grillet wrote:
> > src/python/lib/tox.ini
> > Lines 19 (patched)
> > 
> >
> > As this is here, the line `pylint==1.6.4` in 
> > support/pip-requirements.txt should be removed.

the python files within support/ still require pylint to be installed in 
support/.virtualenv, since they are linted using the pylint installed in 
support/.virtualenv directly.


- Eric


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


On March 14, 2018, 3:05 p.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64970/
> ---
> 
> (Updated March 14, 2018, 3:05 p.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/tests/__init__.py PRE-CREATION 
>   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/6/
> 
> 
> 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: Use tox for linting and testing code living uder src/python.

2018-04-06 Thread Armand Grillet 

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


Fix it, then Ship it!





src/python/lib/tox.ini
Lines 19 (patched)


As this is here, the line `pylint==1.6.4` in support/pip-requirements.txt 
should be removed.


- Armand Grillet


On March 14, 2018, 3:05 p.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64970/
> ---
> 
> (Updated March 14, 2018, 3:05 p.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/tests/__init__.py PRE-CREATION 
>   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/6/
> 
> 
> 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: Use tox for linting and testing code living uder src/python.

2018-03-15 Thread Mesos Reviewbot 

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



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 March 14, 2018, 3:05 p.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64970/
> ---
> 
> (Updated March 14, 2018, 3:05 p.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/tests/__init__.py PRE-CREATION 
>   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/6/
> 
> 
> 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: Use tox for linting and testing code living uder src/python.

2018-03-14 Thread Mesos Reviewbot Windows 

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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['64970']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

- 
[libprocess-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64970/logs/libprocess-tests-stdout.log):

```
[ RUN  ] TimeTest.Output
[   OK ] TimeTest.Output (0 ms)
[--] 4 tests from TimeTest (4 ms total)

[--] 3 tests from TimeSeriesTest
[ RUN  ] TimeSeriesTest.Set
[   OK ] TimeSeriesTest.Set (0 ms)
[ RUN  ] TimeSeriesTest.Sparsify
[   OK ] TimeSeriesTest.Sparsify (1 ms)
[ RUN  ] TimeSeriesTest.Truncate
[   OK ] TimeSeriesTest.Truncate (1 ms)
[--] 3 tests from TimeSeriesTest (5 ms total)

[--] 3 tests from JWTTest
[ RUN  ] JWTTest.Parse
[   OK ] JWTTest.Parse (7 ms)
[ RUN  ] JWTTest.Create
[   OK ] JWTTest.Create (1 ms)
[ RUN  ] JWTTest.Stringify
[   OK ] JWTTest.Stringify (0 ms)
[--] 3 tests from JWTTest (11 ms total)

[--] 1 test from SSL
[ RUN  ] SSL.Disabled
[   OK ] SSL.Disabled (9 ms)
[--] 1 test from SSL (9 ms total)

[--] 17 tests from SSLTest
[ RUN  ] SSLTest.SSLSocket
```

- 
[libprocess-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64970/logs/libprocess-tests-stderr.log):

```
ABORT: 
(D:\DCOS\mesos\mesos\3rdparty\libprocess\include\process/ssl/gtest.hpp:171): 
Could not generate certificate: Failed to set common name: 
X509_NAME_add_entry_by_txt
```

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64970/logs/mesos-tests-stdout.log):

```
[   OK ] ContentType/SchedulerTest.Revive/1 (259 ms)
[ RUN  ] ContentType/SchedulerTest.Suppress/0
[   OK ] ContentType/SchedulerTest.Suppress/0 (242 ms)
[ RUN  ] ContentType/SchedulerTest.Suppress/1
[   OK ] ContentType/SchedulerTest.Suppress/1 (246 ms)
[ RUN  ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/0
[   OK ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/0 (239 ms)
[ RUN  ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/1
[   OK ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/1 (254 ms)
[ RUN  ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/0
[   OK ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/0 (285 
ms)
[ RUN  ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/1
[   OK ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/1 (320 
ms)
[ RUN  ] ContentType/SchedulerTest.Message/0
[   OK ] ContentType/SchedulerTest.Message/0 (315 ms)
[ RUN  ] ContentType/SchedulerTest.Message/1
[   OK ] ContentType/SchedulerTest.Message/1 (319 ms)
[ RUN  ] ContentType/SchedulerTest.Request/0
[   OK ] ContentType/SchedulerTest.Request/0 (100 ms)
[ RUN  ] ContentType/SchedulerTest.Request/1
[   OK ] ContentType/SchedulerTest.Request/1 (101 ms)
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/0
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/0 (90 ms)
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/1
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/1 (85 ms)
[--] 32 tests from ContentType/SchedulerTest (15049 ms total)

[--] 4 tests from ContentTypeAndSSLConfig/SchedulerSSLTest
[ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0
```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64970/logs/mesos-tests-stderr.log):

```
I0314 15:57:02.337544  9132 master.cpp:1673] Recovering from registrar
I0314 15:57:02.338690 10996 registrar.cpp:391] Successfully fetched the 
registry (0B) in 1.145088ms
I0314 15:57:02.338690 10996 registrar.cpp:495] Applied 1 operations in 0ns; 
attempting to update the registry
I0314 15:57:02.339540  1140 registrar.cpp:552] Successfully updated the 
registry in 850944ns
I0314 15:57:02.339540  1140 registrar.cpp:424] Successfully recovered registrar
I0314 15:57:02.340536  9100 master.cpp:1787] Recovered 0 agents from the 
registry (239B); allowing 10mins for agents to reregister
I0314 15:57:02.356540 11656 scheduler.cpp:188] Version: 1.6.0
I0314 15:57:02.357491  7076 scheduler.cpp:311] Using default 'basic' HTTP 
authenticatee
I0314 15:57:02.357491  6628 scheduler.cpp:494] New master detected at 
master@10.3.1.9:51848
I0314 15:57:02.366562 12228 scheduler.cpp:468] Re-detecting master
I0314 15:57:02.367517 12228 scheduler.cpp:494] New master detected at 
master@10.3.1.9:51848
I0314

Re: Review Request 64970: Use tox for linting and testing code living uder src/python.

2018-03-14 Thread Eric Chung 

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

(Updated March 14, 2018, 3:05 p.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 (updated)
-

  src/python/cli_new/tests/__init__.py PRE-CREATION 
  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/6/

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


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: Use tox for linting and testing code living uder src/python.

2018-03-14 Thread Eric Chung 


> On March 1, 2018, 11:20 p.m., Kevin Klues wrote:
> > support/mesos-style.py
> > Lines 413-414 (patched)
> > 
> >
> > This could probably be one line.

```
* Module mesos-style
C:409, 0: Line too long (82/80) (line-too-long)
C:442, 0: Line too long (91/80) (line-too-long)
```


- Eric


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


On March 14, 2018, 3:02 p.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64970/
> ---
> 
> (Updated March 14, 2018, 3:02 p.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/tests/__init__.py PRE-CREATION 
>   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/5/
> 
> 
> 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: Use tox for linting and testing code living uder src/python.

2018-03-14 Thread Eric Chung 

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

(Updated March 14, 2018, 3:02 p.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 (updated)
-

  src/python/cli_new/tests/__init__.py PRE-CREATION 
  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/5/

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


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: Use tox for linting and testing code living uder src/python.

2018-03-01 Thread Kevin Klues 

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




src/python/cli_new/tox.ini
Lines 1-22 (patched)


I would pull the addition of this file out to it's own separate commit 
right after the commit changing `lib/tox.ini`.



src/python/cli_new/tox.ini
Lines 22 (patched)


Let's make `tests` a module and add it in here as well. Also let's 
explicitly add the `.py` files under `bin` to this list as well. That way all 
relevant python files will be linted by default when invoking tox directly here 
instead of via `support/mesos-style.py`



src/python/lib/tox.ini
Line 1 (original), 1 (patched)


I would pull the changing of this file out to it's own separate commit at 
the beginning of the chain of commits for this RR.



src/python/lib/tox.ini
Lines 22 (patched)


Let's add `setup.py` to this list by default as well.



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


s/run/lint/g



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


If we decide to add `@staticmethod` here, we should do it on all functions 
that qualify for this in the entire file.



support/mesos-style.py
Line 374 (original), 375-378 (patched)


Thi docstring is incorrect.



support/mesos-style.py
Lines 379-394 (patched)


Let's change this to:
```
   tox_cmd = [os.path.dirname(__file__), '.virtualenv', 'bin', 'tox')]
   tox_cmd += ['-c', configfile]
   if tox_env is not None:
   tox_cmd += ['-e', tox_env]
   if recreate:
   tox_cmd += ['--recreate']
   tox_cmd += ['--'] + args
```

I understand that using a generator would make it immutable and doing so is 
likely preferable in many circumstances, but I think here is reduces 
readability and is inconsistent with the rest of the code base.



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


Let's consider doing a blanket addition of @staticmethod where appropriate 
in a separate commit so that the code always remains consistent.



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


Let's consider renaming this function. Since it doesn't actually take the 
same parameters as `run_lint()`, I think we can be more descriptive in its 
naming.



support/mesos-style.py
Lines 384-385 (original), 409-410 (patched)


I think that this doc string is incorrect now.

It should also indicate what the return value of this function means.



support/mesos-style.py
Lines 413-414 (patched)


This could probably be one line.



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


This should be in it's own isolated commit before this review request with 
a good explanation of what it's doing and why it's necessary.



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


Let's consider renaming this function. Since it doesn't actually take the 
same parameters as `run_lint()`, I think we can be more descriptive in its 
naming.


- Kevin Klues


On March 1, 2018, 10:34 p.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64970/
> ---
> 
> (Updated March 1, 2018, 10:34 p.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

Re: Review Request 64970: Use tox for linting and testing code living uder src/python.

2018-03-01 Thread Eric Chung 

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

(Updated March 1, 2018, 10:34 p.m.)


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


Summary (updated)
-

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


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: Use tox for linting and testing code living uder src/python.

2018-02-02 Thread Eric Chung 

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




src/python/cli_new/tox.ini
Lines 21 (patched)


fix --cov



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


consider using __file__ instead


- Eric Chung


On Jan. 19, 2018, 4:29 a.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64970/
> ---
> 
> (Updated Jan. 19, 2018, 4:29 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 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
>   support/mesos-style.py 1b34ea2d9afa8f17b545841cea7a6853a6e18144 
> 
> 
> Diff: https://reviews.apache.org/r/64970/diff/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: Use tox for linting and testing code living uder src/python.

2018-01-18 Thread Mesos Reviewbot Windows 

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



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 Jan. 18, 2018, 11:46 p.m., Eric Chung wrote:
> 
> ---
> 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
> ---
> 
> 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 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
>   support/mesos-style.py 1b34ea2d9afa8f17b545841cea7a6853a6e18144 
> 
> 
> Diff: https://reviews.apache.org/r/64970/diff/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
> 
>