You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Mark Chu-Carroll <mc...@twopensource.com> on 2014/08/11 22:12:00 UTC

Review Request 24573: Fix bug in "aurora job inspect".

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

Review request for Aurora, David McLaughlin and Bill Farner.


Bugs: aurora-642
    https://issues.apache.org/jira/browse/aurora-642


Repository: aurora


Description
-------

Inspect was using context.print_out() to add blank lines, but the
print_out method takes a mandatory string parameter.


Diffs
-----

  src/main/python/apache/aurora/client/cli/jobs.py 3cb39b232648d69615bbdfedc4d81eaf7ece7938 

Diff: https://reviews.apache.org/r/24573/diff/


Testing
-------


Thanks,

Mark Chu-Carroll


Re: Review Request 24573: Fix bug in "aurora job inspect".

Posted by Mark Chu-Carroll <mc...@twopensource.com>.

> On Aug. 11, 2014, 4:18 p.m., Bill Farner wrote:
> > Issue points out lack of test coverage, can you add that with the fix?

Sure. I think for this, it makes sense to put it in end-to-end. (unit tests in the client focus on verifying that API calls got made correctly; this seems more like a system-behavior test like what we do in the e2e.)


- Mark


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


On Aug. 11, 2014, 4:12 p.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24573/
> -----------------------------------------------------------
> 
> (Updated Aug. 11, 2014, 4:12 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: aurora-642
>     https://issues.apache.org/jira/browse/aurora-642
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Inspect was using context.print_out() to add blank lines, but the
> print_out method takes a mandatory string parameter.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/client/cli/jobs.py 3cb39b232648d69615bbdfedc4d81eaf7ece7938 
> 
> Diff: https://reviews.apache.org/r/24573/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>


Re: Review Request 24573: Fix bug in "aurora job inspect".

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24573/#review50224
-----------------------------------------------------------


Issue points out lack of test coverage, can you add that with the fix?

- Bill Farner


On Aug. 11, 2014, 8:12 p.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24573/
> -----------------------------------------------------------
> 
> (Updated Aug. 11, 2014, 8:12 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: aurora-642
>     https://issues.apache.org/jira/browse/aurora-642
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Inspect was using context.print_out() to add blank lines, but the
> print_out method takes a mandatory string parameter.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/client/cli/jobs.py 3cb39b232648d69615bbdfedc4d81eaf7ece7938 
> 
> Diff: https://reviews.apache.org/r/24573/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>


Re: Review Request 24573: Fix bug in "aurora job inspect".

Posted by David McLaughlin <da...@dmclaughlin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24573/#review50498
-----------------------------------------------------------

Ship it!


Ship It!

- David McLaughlin


On Aug. 13, 2014, 7:27 p.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24573/
> -----------------------------------------------------------
> 
> (Updated Aug. 13, 2014, 7:27 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: aurora-642
>     https://issues.apache.org/jira/browse/aurora-642
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Inspect was using context.print_out() to add blank lines, but the
> print_out method takes a mandatory string parameter.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/client/cli/__init__.py 08590d0895a2b4082c6929a261b36c8a1a82ac97 
>   src/main/python/apache/aurora/client/cli/jobs.py 3cb39b232648d69615bbdfedc4d81eaf7ece7938 
>   src/test/python/apache/aurora/client/cli/BUILD 3c88ed7cf9f654bbbd80d1d44aa1dd1c8655e378 
>   src/test/python/apache/aurora/client/cli/test_inspect.py PRE-CREATION 
>   src/test/python/apache/aurora/client/cli/test_kill.py ee64908855a4960f44ce96c810e69dd105d2ce5d 
>   src/test/python/apache/aurora/client/cli/util.py 5d2e72d9a475cbb7821ca466d13c195d5eb942c1 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh ea5ae8d0004aea42b7e750eb52c34a636e3a7998 
> 
> Diff: https://reviews.apache.org/r/24573/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>


Re: Review Request 24573: Fix bug in "aurora job inspect".

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24573/#review50615
-----------------------------------------------------------

Ship it!


Ship It!

- Bill Farner


On Aug. 14, 2014, 5:30 p.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24573/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2014, 5:30 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: aurora-642
>     https://issues.apache.org/jira/browse/aurora-642
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Inspect was using context.print_out() to add blank lines, but the
> print_out method takes a mandatory string parameter.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/client/cli/__init__.py 08590d0895a2b4082c6929a261b36c8a1a82ac97 
>   src/main/python/apache/aurora/client/cli/jobs.py 3cb39b232648d69615bbdfedc4d81eaf7ece7938 
>   src/test/python/apache/aurora/client/cli/BUILD 3c88ed7cf9f654bbbd80d1d44aa1dd1c8655e378 
>   src/test/python/apache/aurora/client/cli/test_inspect.py PRE-CREATION 
>   src/test/python/apache/aurora/client/cli/util.py 5d2e72d9a475cbb7821ca466d13c195d5eb942c1 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh ea5ae8d0004aea42b7e750eb52c34a636e3a7998 
> 
> Diff: https://reviews.apache.org/r/24573/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>


Re: Review Request 24573: Fix bug in "aurora job inspect".

Posted by Mark Chu-Carroll <mc...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24573/
-----------------------------------------------------------

(Updated Aug. 14, 2014, 1:30 p.m.)


Review request for Aurora, David McLaughlin and Bill Farner.


Changes
-------

Address Bill's review, and run checkstyle.


Bugs: aurora-642
    https://issues.apache.org/jira/browse/aurora-642


Repository: aurora


Description (updated)
-------

Inspect was using context.print_out() to add blank lines, but the
print_out method takes a mandatory string parameter.


Diffs (updated)
-----

  src/main/python/apache/aurora/client/cli/__init__.py 08590d0895a2b4082c6929a261b36c8a1a82ac97 
  src/main/python/apache/aurora/client/cli/jobs.py 3cb39b232648d69615bbdfedc4d81eaf7ece7938 
  src/test/python/apache/aurora/client/cli/BUILD 3c88ed7cf9f654bbbd80d1d44aa1dd1c8655e378 
  src/test/python/apache/aurora/client/cli/test_inspect.py PRE-CREATION 
  src/test/python/apache/aurora/client/cli/util.py 5d2e72d9a475cbb7821ca466d13c195d5eb942c1 
  src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh ea5ae8d0004aea42b7e750eb52c34a636e3a7998 

Diff: https://reviews.apache.org/r/24573/diff/


Testing
-------


Thanks,

Mark Chu-Carroll


Re: Review Request 24573: Fix bug in "aurora job inspect".

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24573/#review50583
-----------------------------------------------------------



src/test/python/apache/aurora/client/cli/BUILD
<https://reviews.apache.org/r/24573/#comment88445>

    -2 indent, remove extra newline after



src/test/python/apache/aurora/client/cli/test_inspect.py
<https://reviews.apache.org/r/24573/#comment88450>

    missing license header



src/test/python/apache/aurora/client/cli/test_inspect.py
<https://reviews.apache.org/r/24573/#comment88446>

    Leave no TODO unassigned: TODO(markcc)



src/test/python/apache/aurora/client/cli/test_inspect.py
<https://reviews.apache.org/r/24573/#comment88447>

    remove?



src/test/python/apache/aurora/client/cli/test_inspect.py
<https://reviews.apache.org/r/24573/#comment88448>

    remove comment?



src/test/python/apache/aurora/client/cli/test_kill.py
<https://reviews.apache.org/r/24573/#comment88449>

    revert?  shouldn't classes be separated by 2 blank lines?


- Bill Farner


On Aug. 13, 2014, 7:27 p.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24573/
> -----------------------------------------------------------
> 
> (Updated Aug. 13, 2014, 7:27 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: aurora-642
>     https://issues.apache.org/jira/browse/aurora-642
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Inspect was using context.print_out() to add blank lines, but the
> print_out method takes a mandatory string parameter.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/client/cli/__init__.py 08590d0895a2b4082c6929a261b36c8a1a82ac97 
>   src/main/python/apache/aurora/client/cli/jobs.py 3cb39b232648d69615bbdfedc4d81eaf7ece7938 
>   src/test/python/apache/aurora/client/cli/BUILD 3c88ed7cf9f654bbbd80d1d44aa1dd1c8655e378 
>   src/test/python/apache/aurora/client/cli/test_inspect.py PRE-CREATION 
>   src/test/python/apache/aurora/client/cli/test_kill.py ee64908855a4960f44ce96c810e69dd105d2ce5d 
>   src/test/python/apache/aurora/client/cli/util.py 5d2e72d9a475cbb7821ca466d13c195d5eb942c1 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh ea5ae8d0004aea42b7e750eb52c34a636e3a7998 
> 
> Diff: https://reviews.apache.org/r/24573/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>


Re: Review Request 24573: Fix bug in "aurora job inspect".

Posted by Mark Chu-Carroll <mc...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24573/
-----------------------------------------------------------

(Updated Aug. 13, 2014, 3:27 p.m.)


Review request for Aurora, David McLaughlin and Bill Farner.


Changes
-------

Add tests.


Bugs: aurora-642
    https://issues.apache.org/jira/browse/aurora-642


Repository: aurora


Description
-------

Inspect was using context.print_out() to add blank lines, but the
print_out method takes a mandatory string parameter.


Diffs (updated)
-----

  src/main/python/apache/aurora/client/cli/__init__.py 08590d0895a2b4082c6929a261b36c8a1a82ac97 
  src/main/python/apache/aurora/client/cli/jobs.py 3cb39b232648d69615bbdfedc4d81eaf7ece7938 
  src/test/python/apache/aurora/client/cli/BUILD 3c88ed7cf9f654bbbd80d1d44aa1dd1c8655e378 
  src/test/python/apache/aurora/client/cli/test_inspect.py PRE-CREATION 
  src/test/python/apache/aurora/client/cli/test_kill.py ee64908855a4960f44ce96c810e69dd105d2ce5d 
  src/test/python/apache/aurora/client/cli/util.py 5d2e72d9a475cbb7821ca466d13c195d5eb942c1 
  src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh ea5ae8d0004aea42b7e750eb52c34a636e3a7998 

Diff: https://reviews.apache.org/r/24573/diff/


Testing
-------


Thanks,

Mark Chu-Carroll


Re: Review Request 24573: Fix bug in "aurora job inspect".

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24573/#review50223
-----------------------------------------------------------


Issue points out lack of test coverage, can you add that with the fix?

- Bill Farner


On Aug. 11, 2014, 8:12 p.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24573/
> -----------------------------------------------------------
> 
> (Updated Aug. 11, 2014, 8:12 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: aurora-642
>     https://issues.apache.org/jira/browse/aurora-642
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Inspect was using context.print_out() to add blank lines, but the
> print_out method takes a mandatory string parameter.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/client/cli/jobs.py 3cb39b232648d69615bbdfedc4d81eaf7ece7938 
> 
> Diff: https://reviews.apache.org/r/24573/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>