You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Zameer Manji <zm...@apache.org> on 2015/04/06 22:30:38 UTC

Review Request 32900: Remove url related methods out of AuroraCommandContext

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

Review request for Aurora and Bill Farner.


Repository: aurora


Description
-------

This is a refactor of AuroraCommandContext which removes all url related methods out of it. The objective of this refactor is to remove functionality from AuroraCommandContext to allow for easier testing of commands. This commit also adds two tests for commands which were using the url related functionality but lacked test coverage.


Diffs
-----

  src/main/python/apache/aurora/client/base.py 0c8e97e0b26b0c4ede77873eaae76ebe7bf36a4a 
  src/main/python/apache/aurora/client/cli/context.py 51c7d24dca664e476e62f1864d095416dfab70e4 
  src/main/python/apache/aurora/client/cli/cron.py 3416c8e1932056725880f2007b60d77112759428 
  src/main/python/apache/aurora/client/cli/jobs.py 8f349c09637c16e2499e85f2dc96eb7ccffd0aaf 
  src/main/python/apache/aurora/client/cli/update.py PRE-CREATION 
  src/test/python/apache/aurora/client/cli/test_create.py 31fa56f5edcfc97903725ab27ccc12c6a8f39ffc 
  src/test/python/apache/aurora/client/cli/test_cron.py f488432cd68cc68fab8fce968e8605625ea3f56a 
  src/test/python/apache/aurora/client/cli/test_kill.py e3a366bf67074e50787394cad58d5e01359b641e 
  src/test/python/apache/aurora/client/cli/test_open.py c20649f5cada241d0f6e9ae5f88d300eac073517 
  src/test/python/apache/aurora/client/cli/test_restart.py 92aefe612dd59df75188fd7fc8cf080c9a878dde 
  src/test/python/apache/aurora/client/cli/util.py 95a2123e127c9811fd2305e71cfc5c7c4376f904 

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


Testing
-------

./pants test.pytest --no-fast src/test/python/apache/aurora/client/cli::


Thanks,

Zameer Manji


Re: Review Request 32900: Remove url related methods out of AuroraCommandContext

Posted by Zameer Manji <zm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32900/
-----------------------------------------------------------

(Updated April 7, 2015, 12:44 p.m.)


Review request for Aurora and Bill Farner.


Changes
-------

Update diff with Bill's feedback.


Repository: aurora


Description
-------

This is a refactor of AuroraCommandContext which removes all url related methods out of it. The objective of this refactor is to remove functionality from AuroraCommandContext to allow for easier testing of commands. This commit also adds two tests for commands which were using the url related functionality but lacked test coverage.


Diffs (updated)
-----

  src/main/python/apache/aurora/client/base.py c72f2f700fd63f77f920d1e0e7f1183f08bd3906 
  src/main/python/apache/aurora/client/cli/context.py e75c6cb6c29727654e6bd06e4391abf2d7ae0f0a 
  src/main/python/apache/aurora/client/cli/cron.py 732135fa7c149140aed4d5c9ae0b8a2e4608c388 
  src/main/python/apache/aurora/client/cli/jobs.py 2d82942b70e6bc04c246809cb34197978c83e5b4 
  src/main/python/apache/aurora/client/cli/update.py 5f98fc27480e4ee104ce03acfbf091772e9ac7e5 
  src/test/python/apache/aurora/client/cli/test_create.py 57970c467bc5223467d78267bcd160f2d12f9116 
  src/test/python/apache/aurora/client/cli/test_cron.py 9fa176e35272a0b2eb30a246081e1b3526d207fb 
  src/test/python/apache/aurora/client/cli/test_kill.py 69d84022291617f3a28d269319c7135363e900ce 
  src/test/python/apache/aurora/client/cli/test_open.py 92f9c3d2c916e0dcbbba508ea5e5b756499631da 
  src/test/python/apache/aurora/client/cli/test_restart.py fb5491dc3e2ac3fd687edb1c819c4b399800e27a 
  src/test/python/apache/aurora/client/cli/util.py 291186b75103b570188b2782db543cff4d112273 

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


Testing
-------

./pants test.pytest --no-fast src/test/python/apache/aurora/client/cli::


Thanks,

Zameer Manji


Re: Review Request 32900: Remove url related methods out of AuroraCommandContext

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32900/#review79052
-----------------------------------------------------------

Ship it!


Master (5587bce) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot retry"

- Aurora ReviewBot


On April 6, 2015, 8:30 p.m., Zameer Manji wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32900/
> -----------------------------------------------------------
> 
> (Updated April 6, 2015, 8:30 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is a refactor of AuroraCommandContext which removes all url related methods out of it. The objective of this refactor is to remove functionality from AuroraCommandContext to allow for easier testing of commands. This commit also adds two tests for commands which were using the url related functionality but lacked test coverage.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/client/base.py 0c8e97e0b26b0c4ede77873eaae76ebe7bf36a4a 
>   src/main/python/apache/aurora/client/cli/context.py 51c7d24dca664e476e62f1864d095416dfab70e4 
>   src/main/python/apache/aurora/client/cli/cron.py 3416c8e1932056725880f2007b60d77112759428 
>   src/main/python/apache/aurora/client/cli/jobs.py 8f349c09637c16e2499e85f2dc96eb7ccffd0aaf 
>   src/main/python/apache/aurora/client/cli/update.py PRE-CREATION 
>   src/test/python/apache/aurora/client/cli/test_create.py 31fa56f5edcfc97903725ab27ccc12c6a8f39ffc 
>   src/test/python/apache/aurora/client/cli/test_cron.py f488432cd68cc68fab8fce968e8605625ea3f56a 
>   src/test/python/apache/aurora/client/cli/test_kill.py e3a366bf67074e50787394cad58d5e01359b641e 
>   src/test/python/apache/aurora/client/cli/test_open.py c20649f5cada241d0f6e9ae5f88d300eac073517 
>   src/test/python/apache/aurora/client/cli/test_restart.py 92aefe612dd59df75188fd7fc8cf080c9a878dde 
>   src/test/python/apache/aurora/client/cli/util.py 95a2123e127c9811fd2305e71cfc5c7c4376f904 
> 
> Diff: https://reviews.apache.org/r/32900/diff/
> 
> 
> Testing
> -------
> 
> ./pants test.pytest --no-fast src/test/python/apache/aurora/client/cli::
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>


Re: Review Request 32900: Remove url related methods out of AuroraCommandContext

Posted by Zameer Manji <zm...@apache.org>.

> On April 6, 2015, 5:16 p.m., Bill Farner wrote:
> > src/main/python/apache/aurora/client/cli/cron.py, line 116
> > <https://reviews.apache.org/r/32900/diff/1/?file=918253#file918253line116>
> >
> >     inline `url`.  ditto several other places in this diff.

Done.


> On April 6, 2015, 5:16 p.m., Bill Farner wrote:
> > src/test/python/apache/aurora/client/cli/test_create.py, line 178
> > <https://reviews.apache.org/r/32900/diff/1/?file=918256#file918256line178>
> >
> >     should be +4 indent, here and elsewhere in this diff.

Done.


> On April 6, 2015, 5:16 p.m., Bill Farner wrote:
> > src/main/python/apache/aurora/client/base.py, line 176
> > <https://reviews.apache.org/r/32900/diff/1/?file=918251#file918251line176>
> >
> >     i have no strong opinion, but it feels odd to add `update_id` to this signature.  consider keeping this signature, and implementing the suffix addition in `get_update_page`.

I did not revert the signature because update's are now first class entities in Aurora including our URL structure. This function reflects that.


- Zameer


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


On April 7, 2015, 12:44 p.m., Zameer Manji wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32900/
> -----------------------------------------------------------
> 
> (Updated April 7, 2015, 12:44 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is a refactor of AuroraCommandContext which removes all url related methods out of it. The objective of this refactor is to remove functionality from AuroraCommandContext to allow for easier testing of commands. This commit also adds two tests for commands which were using the url related functionality but lacked test coverage.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/client/base.py c72f2f700fd63f77f920d1e0e7f1183f08bd3906 
>   src/main/python/apache/aurora/client/cli/context.py e75c6cb6c29727654e6bd06e4391abf2d7ae0f0a 
>   src/main/python/apache/aurora/client/cli/cron.py 732135fa7c149140aed4d5c9ae0b8a2e4608c388 
>   src/main/python/apache/aurora/client/cli/jobs.py 2d82942b70e6bc04c246809cb34197978c83e5b4 
>   src/main/python/apache/aurora/client/cli/update.py 5f98fc27480e4ee104ce03acfbf091772e9ac7e5 
>   src/test/python/apache/aurora/client/cli/test_create.py 57970c467bc5223467d78267bcd160f2d12f9116 
>   src/test/python/apache/aurora/client/cli/test_cron.py 9fa176e35272a0b2eb30a246081e1b3526d207fb 
>   src/test/python/apache/aurora/client/cli/test_kill.py 69d84022291617f3a28d269319c7135363e900ce 
>   src/test/python/apache/aurora/client/cli/test_open.py 92f9c3d2c916e0dcbbba508ea5e5b756499631da 
>   src/test/python/apache/aurora/client/cli/test_restart.py fb5491dc3e2ac3fd687edb1c819c4b399800e27a 
>   src/test/python/apache/aurora/client/cli/util.py 291186b75103b570188b2782db543cff4d112273 
> 
> Diff: https://reviews.apache.org/r/32900/diff/
> 
> 
> Testing
> -------
> 
> ./pants test.pytest --no-fast src/test/python/apache/aurora/client/cli::
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>


Re: Review Request 32900: Remove url related methods out of AuroraCommandContext

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

Ship it!



src/main/python/apache/aurora/client/base.py
<https://reviews.apache.org/r/32900/#comment128184>

    i have no strong opinion, but it feels odd to add `update_id` to this signature.  consider keeping this signature, and implementing the suffix addition in `get_update_page`.



src/main/python/apache/aurora/client/cli/cron.py
<https://reviews.apache.org/r/32900/#comment128183>

    inline `url`.  ditto several other places in this diff.



src/test/python/apache/aurora/client/cli/test_create.py
<https://reviews.apache.org/r/32900/#comment128185>

    should be +4 indent, here and elsewhere in this diff.


- Bill Farner


On April 6, 2015, 8:30 p.m., Zameer Manji wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32900/
> -----------------------------------------------------------
> 
> (Updated April 6, 2015, 8:30 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is a refactor of AuroraCommandContext which removes all url related methods out of it. The objective of this refactor is to remove functionality from AuroraCommandContext to allow for easier testing of commands. This commit also adds two tests for commands which were using the url related functionality but lacked test coverage.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/client/base.py 0c8e97e0b26b0c4ede77873eaae76ebe7bf36a4a 
>   src/main/python/apache/aurora/client/cli/context.py 51c7d24dca664e476e62f1864d095416dfab70e4 
>   src/main/python/apache/aurora/client/cli/cron.py 3416c8e1932056725880f2007b60d77112759428 
>   src/main/python/apache/aurora/client/cli/jobs.py 8f349c09637c16e2499e85f2dc96eb7ccffd0aaf 
>   src/main/python/apache/aurora/client/cli/update.py PRE-CREATION 
>   src/test/python/apache/aurora/client/cli/test_create.py 31fa56f5edcfc97903725ab27ccc12c6a8f39ffc 
>   src/test/python/apache/aurora/client/cli/test_cron.py f488432cd68cc68fab8fce968e8605625ea3f56a 
>   src/test/python/apache/aurora/client/cli/test_kill.py e3a366bf67074e50787394cad58d5e01359b641e 
>   src/test/python/apache/aurora/client/cli/test_open.py c20649f5cada241d0f6e9ae5f88d300eac073517 
>   src/test/python/apache/aurora/client/cli/test_restart.py 92aefe612dd59df75188fd7fc8cf080c9a878dde 
>   src/test/python/apache/aurora/client/cli/util.py 95a2123e127c9811fd2305e71cfc5c7c4376f904 
> 
> Diff: https://reviews.apache.org/r/32900/diff/
> 
> 
> Testing
> -------
> 
> ./pants test.pytest --no-fast src/test/python/apache/aurora/client/cli::
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>