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

2015-04-07 Thread Zameer Manji 


> On April 6, 2015, 5:16 p.m., Bill Farner wrote:
> > src/main/python/apache/aurora/client/cli/cron.py, line 116
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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

2015-04-07 Thread Zameer Manji 

---
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

2015-04-06 Thread Bill Farner 

---
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


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


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



src/test/python/apache/aurora/client/cli/test_create.py


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
> 
>

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

2015-04-06 Thread Aurora ReviewBot 

---
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
> 
>