> On Nov. 16, 2015, 9:27 a.m., Joshua Cohen wrote:
> > src/test/python/apache/aurora/admin/test_admin.py, lines 158-160
> > <https://reviews.apache.org/r/40310/diff/2/?file=1125865#file1125865line158>
> >
> > Why kill this? It seems like the intention was to ensure that set quota
> > is called with the correct types, but the assert above still passes if we
> > were to do `assert_called_with(role, 24, 4001.0, 6001.0)`, thus the
> > explicit type checks were necessary?
Ah right - I was thinking this was a tautology but its not, `24.0 == 24` is
`True`.
This action on my part was triggered by:
```
10:08:08 00:00 [compile]
10:08:08 00:00 [python-eval]
10:08:08 00:00 [pythonstyle]
Invalidated 9 targets.
E721:ERROR src/test/python/apache/aurora/admin/test_admin.py:158 do not
compare types, use 'isinstance()'
| assert type(api.set_quota.call_args[0][1]) == type(float())
```
I've switched to isinstance which also checks what you want; ie:
`isinstance(24.0, int)` fails, etc.
> On Nov. 16, 2015, 9:27 a.m., Joshua Cohen wrote:
> > pants.ini, line 19
> > <https://reviews.apache.org/r/40310/diff/2/?file=1125858#file1125858line19>
> >
> > nit: fix indent?
You'll need to give me exact direction, -1 or +N or pull up onto the end of the
prior line. There must be some whitespace indent per ConfigParser rules to
keep this as a single value.
> On Nov. 16, 2015, 9:27 a.m., Joshua Cohen wrote:
> > pants.ini, line 77
> > <https://reviews.apache.org/r/40310/diff/2/?file=1125858#file1125858line77>
> >
> > or maybe this identation style is intentional/correct?
Ah - yes, the indent is intentional / required. The indent is required as
explained above, but the amount of indent is a style thing and your call.
> On Nov. 16, 2015, 9:27 a.m., Joshua Cohen wrote:
> > src/main/python/apache/aurora/client/api/__init__.py, lines 283-284
> > <https://reviews.apache.org/r/40310/diff/2/?file=1125860#file1125860line283>
> >
> > alternately this should validate and save the interim var?
> >
> > return Restarter(
> > job_key,
> > updater_config,
> > ...
> > self._scheduler_proxy).restart(instances)
I couldn't follow the comment, but did change the style to the one you
demonstrated.
- John
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40310/#review106674
-----------------------------------------------------------
On Nov. 15, 2015, 9:02 p.m., John Sirois wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40310/
> -----------------------------------------------------------
>
> (Updated Nov. 15, 2015, 9:02 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Joe Smith, Maxim Khutornenko, and
> Zameer Manji.
>
>
> Bugs: AURORA-1532
> https://issues.apache.org/jira/browse/AURORA-1532
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This upgrades to pants 0.0.58 to pick up the newly split off pants
> python checks contrib plugin. Release notes are here:
> https://pypi.python.org/pypi/pantsbuild.pants/0.0.58
>
> The plugin provides both python checkstyle (`compile.pythonstyle`), and
> a python eval task (`compile.python-eval`). The `python-eval` is turned
> off since at least one of the Aurora python targets has files that have
> side-effects upon import (a repl is started).
>
> Now style checks run before compile (and thus before tests) and they
> benefit from fingerprinting; ie: if you test your changes, those tests
> will run style checks and when you go to commit, those checks will not
> be re-run by the commit hook (although files you did not test will still
> need to be checked).
>
> A few production files were fixed up according to style failures coming
> from:
> + no space after comment opening '#'
> + unused variables
> + mis-aligned hanging closing parens.
>
>
> Diffs
> -----
>
> build-support/hooks/pre-commit 619fa9e245be49e4e1f21781c0908cbf744b10ea
> build-support/jenkins/build.sh 41a392162f62236771ccbef5c9f94bf84b899f26
> build-support/python/checkstyle 61acc22613acece01580761b25afc7a3edb6b845
> build-support/python/checkstyle-check
> b2bfc5dd71193a8056828e9af05a4c16965f32a1
> pants.ini 319d38e9a7af8055cac5bbce4a6ae0cbb38dc8d0
> src/main/python/apache/aurora/admin/maintenance.py
> 6d94c923ae37bf6b827519d3505b100af306296b
> src/main/python/apache/aurora/client/api/__init__.py
> 6f07a3073a5d422373238619d459fbd09d8adf3d
> src/main/python/apache/aurora/client/cli/client.py
> 297fb588808c1eebc32ac3374265ba986dab3436
> src/main/python/apache/aurora/client/cli/cron.py
> 6376fd014f2a4da29442b5c2c7eb36578b503ba3
> src/main/python/apache/thermos/core/process.py
> fe95cb3be01b47616596bd78cb9a919b2e8bd978
> src/main/python/apache/thermos/monitoring/process_collector_psutil.py
> f1ec5a9050ac60700c4a8afa905bcf12a9bd8a44
> src/test/python/apache/aurora/admin/test_admin.py
> 8e204ab43c6bf69867ea7c32b0a7ba7fb29c0766
> src/test/python/apache/aurora/admin/util.py
> 3570407b51613d0a7b4fde8a4794d88b98e150b5
> src/test/python/apache/aurora/client/cli/test_task.py
> 5432a3d5f7e150b12bd75db0dac7a9018e1c6636
>
> Diff: https://reviews.apache.org/r/40310/diff/
>
>
> Testing
> -------
>
> See the discarded https://reviews.apache.org/r/40219/ for the
> commit-hook check. This version of that RB engages the same code
> and this RB commit was vetted by the same commit-hook.
>
>
> Thanks,
>
> John Sirois
>
>
