Re: Review Request 52087: Fix host maintenance commands to properly initialize the api client.

[Aurora ReviewBot]

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


Ship it!




Master (4745c8c) 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 Sept. 20, 2016, 6:57 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52087/
> ---
> 
> (Updated Sept. 20, 2016, 6:57 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1777
> https://issues.apache.org/jira/browse/AURORA-1777
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fix host maintenance commands to properly initialize the api client.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/admin.py 
> 9fc89a2842d4651ac10aa82118531c450f051712 
>   src/main/python/apache/aurora/admin/admin_util.py 
> 394deb57af9ad8832a02ceab15f33b3c1e5c902b 
>   src/main/python/apache/aurora/admin/host_maintenance.py 
> 677f870c50d1e19d34de36a3c677bb1dcd255ba0 
>   src/main/python/apache/aurora/admin/maintenance.py 
> bf446515222a1246b7fe32acd67315f8145a1212 
>   src/test/python/apache/aurora/admin/test_admin.py 
> f720742c50f774b9f5a41ba57ef251d08d79e8d1 
>   src/test/python/apache/aurora/admin/test_admin_sla.py 
> 54b5a823903b82c2082353ade132eb7b63e518db 
>   src/test/python/apache/aurora/admin/test_admin_util.py 
> f5c8c69c1109d15ee3886fb863014c3285240db1 
>   src/test/python/apache/aurora/admin/util.py 
> d0a915cd8edee6b36d671b41f43a3af4fe751ae7 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
> e36726e0c0154e51bba40704e13eebec5c0f0d65 
> 
> Diff: https://reviews.apache.org/r/52087/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ran e2e tests.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 52087: Fix host maintenance commands to properly initialize the api client.

[Dmitriy Shirchenko]

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


Ship it!




Ship It!

- Dmitriy Shirchenko


On Sept. 20, 2016, 6:57 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52087/
> ---
> 
> (Updated Sept. 20, 2016, 6:57 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1777
> https://issues.apache.org/jira/browse/AURORA-1777
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fix host maintenance commands to properly initialize the api client.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/admin.py 
> 9fc89a2842d4651ac10aa82118531c450f051712 
>   src/main/python/apache/aurora/admin/admin_util.py 
> 394deb57af9ad8832a02ceab15f33b3c1e5c902b 
>   src/main/python/apache/aurora/admin/host_maintenance.py 
> 677f870c50d1e19d34de36a3c677bb1dcd255ba0 
>   src/main/python/apache/aurora/admin/maintenance.py 
> bf446515222a1246b7fe32acd67315f8145a1212 
>   src/test/python/apache/aurora/admin/test_admin.py 
> f720742c50f774b9f5a41ba57ef251d08d79e8d1 
>   src/test/python/apache/aurora/admin/test_admin_sla.py 
> 54b5a823903b82c2082353ade132eb7b63e518db 
>   src/test/python/apache/aurora/admin/test_admin_util.py 
> f5c8c69c1109d15ee3886fb863014c3285240db1 
>   src/test/python/apache/aurora/admin/util.py 
> d0a915cd8edee6b36d671b41f43a3af4fe751ae7 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
> e36726e0c0154e51bba40704e13eebec5c0f0d65 
> 
> Diff: https://reviews.apache.org/r/52087/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ran e2e tests.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 52087: Fix host maintenance commands to properly initialize the api client.

[Joshua Cohen]

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

(Updated Sept. 20, 2016, 6:57 p.m.)


Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Changes
---

Address review feedback.


Bugs: AURORA-1777
https://issues.apache.org/jira/browse/AURORA-1777


Repository: aurora


Description
---

Fix host maintenance commands to properly initialize the api client.


Diffs (updated)
-

  src/main/python/apache/aurora/admin/admin.py 
9fc89a2842d4651ac10aa82118531c450f051712 
  src/main/python/apache/aurora/admin/admin_util.py 
394deb57af9ad8832a02ceab15f33b3c1e5c902b 
  src/main/python/apache/aurora/admin/host_maintenance.py 
677f870c50d1e19d34de36a3c677bb1dcd255ba0 
  src/main/python/apache/aurora/admin/maintenance.py 
bf446515222a1246b7fe32acd67315f8145a1212 
  src/test/python/apache/aurora/admin/test_admin.py 
f720742c50f774b9f5a41ba57ef251d08d79e8d1 
  src/test/python/apache/aurora/admin/test_admin_sla.py 
54b5a823903b82c2082353ade132eb7b63e518db 
  src/test/python/apache/aurora/admin/test_admin_util.py 
f5c8c69c1109d15ee3886fb863014c3285240db1 
  src/test/python/apache/aurora/admin/util.py 
d0a915cd8edee6b36d671b41f43a3af4fe751ae7 
  src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
e36726e0c0154e51bba40704e13eebec5c0f0d65 

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


Testing
---

./build-support/jenkins/build.sh
ran e2e tests.


Thanks,

Joshua Cohen

Re: Review Request 52087: Fix host maintenance commands to properly initialize the api client.

[Joshua Cohen]

> On Sept. 20, 2016, 5:53 p.m., Dmitriy Shirchenko wrote:
> > src/main/python/apache/aurora/admin/admin_util.py, line 272
> > 
> >
> > docstring, please!

Added.


> On Sept. 20, 2016, 5:53 p.m., Dmitriy Shirchenko wrote:
> > src/main/python/apache/aurora/admin/admin_util.py, line 273
> > 
> >
> > isinstance is preferred to type since isinstance keeps track of 
> > inheritance.

Good point, fixed.


> On Sept. 20, 2016, 5:53 p.m., Dmitriy Shirchenko wrote:
> > src/main/python/apache/aurora/admin/admin_util.py, line 276
> > 
> >
> > is there a test for this switch?

Thanks for the nudge, added a few test cases.


> On Sept. 20, 2016, 5:53 p.m., Dmitriy Shirchenko wrote:
> > src/test/python/apache/aurora/admin/test_admin.py, line 232
> > 
> >
> > is there a mock with bypass_leader_redirect = True?

I think there's sufficient coverage for this on the client directly. I don't 
think more coverage here buys us anything more than asserting we pass the value 
on from options which is of dubious value.


- Joshua


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


On Sept. 20, 2016, 6:41 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52087/
> ---
> 
> (Updated Sept. 20, 2016, 6:41 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1777
> https://issues.apache.org/jira/browse/AURORA-1777
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fix host maintenance commands to properly initialize the api client.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/admin.py 
> 9fc89a2842d4651ac10aa82118531c450f051712 
>   src/main/python/apache/aurora/admin/admin_util.py 
> 394deb57af9ad8832a02ceab15f33b3c1e5c902b 
>   src/main/python/apache/aurora/admin/host_maintenance.py 
> 677f870c50d1e19d34de36a3c677bb1dcd255ba0 
>   src/main/python/apache/aurora/admin/maintenance.py 
> bf446515222a1246b7fe32acd67315f8145a1212 
>   src/test/python/apache/aurora/admin/test_admin.py 
> f720742c50f774b9f5a41ba57ef251d08d79e8d1 
>   src/test/python/apache/aurora/admin/test_admin_sla.py 
> 54b5a823903b82c2082353ade132eb7b63e518db 
>   src/test/python/apache/aurora/admin/util.py 
> d0a915cd8edee6b36d671b41f43a3af4fe751ae7 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
> e36726e0c0154e51bba40704e13eebec5c0f0d65 
> 
> Diff: https://reviews.apache.org/r/52087/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ran e2e tests.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 52087: Fix host maintenance commands to properly initialize the api client.

[Joshua Cohen]

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

(Updated Sept. 20, 2016, 6:41 p.m.)


Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Changes
---

+bug


Bugs: AURORA-1777
https://issues.apache.org/jira/browse/AURORA-1777


Repository: aurora


Description
---

Fix host maintenance commands to properly initialize the api client.


Diffs
-

  src/main/python/apache/aurora/admin/admin.py 
9fc89a2842d4651ac10aa82118531c450f051712 
  src/main/python/apache/aurora/admin/admin_util.py 
394deb57af9ad8832a02ceab15f33b3c1e5c902b 
  src/main/python/apache/aurora/admin/host_maintenance.py 
677f870c50d1e19d34de36a3c677bb1dcd255ba0 
  src/main/python/apache/aurora/admin/maintenance.py 
bf446515222a1246b7fe32acd67315f8145a1212 
  src/test/python/apache/aurora/admin/test_admin.py 
f720742c50f774b9f5a41ba57ef251d08d79e8d1 
  src/test/python/apache/aurora/admin/test_admin_sla.py 
54b5a823903b82c2082353ade132eb7b63e518db 
  src/test/python/apache/aurora/admin/util.py 
d0a915cd8edee6b36d671b41f43a3af4fe751ae7 
  src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
e36726e0c0154e51bba40704e13eebec5c0f0d65 

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


Testing
---

./build-support/jenkins/build.sh
ran e2e tests.


Thanks,

Joshua Cohen

Re: Review Request 52087: Fix host maintenance commands to properly initialize the api client.

[Zameer Manji]

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


Ship it!




Don't forget to fill out the `Bugs` portion of the review to link to the JIRA 
ticket.

I greatly appreciate the addition to the e2e test.

- Zameer Manji


On Sept. 20, 2016, 10:32 a.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52087/
> ---
> 
> (Updated Sept. 20, 2016, 10:32 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fix host maintenance commands to properly initialize the api client.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/admin.py 
> 9fc89a2842d4651ac10aa82118531c450f051712 
>   src/main/python/apache/aurora/admin/admin_util.py 
> 394deb57af9ad8832a02ceab15f33b3c1e5c902b 
>   src/main/python/apache/aurora/admin/host_maintenance.py 
> 677f870c50d1e19d34de36a3c677bb1dcd255ba0 
>   src/main/python/apache/aurora/admin/maintenance.py 
> bf446515222a1246b7fe32acd67315f8145a1212 
>   src/test/python/apache/aurora/admin/test_admin.py 
> f720742c50f774b9f5a41ba57ef251d08d79e8d1 
>   src/test/python/apache/aurora/admin/test_admin_sla.py 
> 54b5a823903b82c2082353ade132eb7b63e518db 
>   src/test/python/apache/aurora/admin/util.py 
> d0a915cd8edee6b36d671b41f43a3af4fe751ae7 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
> e36726e0c0154e51bba40704e13eebec5c0f0d65 
> 
> Diff: https://reviews.apache.org/r/52087/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ran e2e tests.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 52087: Fix host maintenance commands to properly initialize the api client.

[Aurora ReviewBot]

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



Master (4745c8c) is red with this patch.
  ./build-support/jenkins/build.sh

 # Create file stdout for capturing output. We 
can't use StringIO mock
 # because TestProcess is running fork.
 with open(os.path.join(td, 'sys_stdout'), 
'w+') as stdout:
   with open(os.path.join(td, 'sys_stderr'), 
'w+') as stderr:
 with mutable_sys():
   sys.stdout, sys.stderr = stdout, 
stderr
 
   p = TestProcess('process', 'echo hello 
world; echo >&2 hello stderr', 0,
   taskpath, sandbox, 
logger_destination=LoggerDestination.BOTH)
   p.start()
   rc = 
wait_for_rc(taskpath.getpath('process_checkpoint'))
 
   assert rc == 0
   # Check log files were created in std 
path with correct content
 > assert_log_content(taskpath, 'stdout', 
'hello world\n')
 
 src/test/python/apache/thermos/core/test_process.py:487: 
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 
 taskpath = 
 log_name = 'stdout'
 expected_content = 'hello world\n'
 
 def assert_log_content(taskpath, log_name, 
expected_content):
   log = 
taskpath.with_filename(log_name).getpath('process_logdir')
   assert os.path.exists(log)
   with open(log, 'r') as fp:
 >   assert fp.read() == expected_content
 E   assert '' == 'hello world\n'
 E + hello world
 
 src/test/python/apache/thermos/core/test_process.py:313: 
AssertionError
  generated xml file: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/415337499eb72578eab327a6487c1f5c9452b3d6.xml
 
  1 failed, 710 passed, 6 skipped, 1 warnings in 
402.35 seconds 
 
FAILURE


17:53:10 07:30   [complete]
   FAILURE


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

- Aurora ReviewBot


On Sept. 20, 2016, 5:32 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52087/
> ---
> 
> (Updated Sept. 20, 2016, 5:32 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fix host maintenance commands to properly initialize the api client.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/admin.py 
> 9fc89a2842d4651ac10aa82118531c450f051712 
>   src/main/python/apache/aurora/admin/admin_util.py 
> 394deb57af9ad8832a02ceab15f33b3c1e5c902b 
>   src/main/python/apache/aurora/admin/host_maintenance.py 
> 677f870c50d1e19d34de36a3c677bb1dcd255ba0 
>   src/main/python/apache/aurora/admin/maintenance.py 
> bf446515222a1246b7fe32acd67315f8145a1212 
>   src/test/python/apache/aurora/admin/test_admin.py 
> f720742c50f774b9f5a41ba57ef251d08d79e8d1 
>   src/test/python/apache/aurora/admin/test_admin_sla.py 
> 54b5a823903b82c2082353ade132eb7b63e518db 
>   src/test/python/apache/aurora/admin/util.py 
> d0a915cd8edee6b36d671b41f43a3af4fe751ae7 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
> e36726e0c0154e51bba40704e13eebec5c0f0d65 
> 
> Diff: https://reviews.apache.org/r/52087/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ran e2e tests.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 52087: Fix host maintenance commands to properly initialize the api client.

[Dmitriy Shirchenko]

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




src/main/python/apache/aurora/admin/admin_util.py (line 272)


docstring, please!



src/main/python/apache/aurora/admin/admin_util.py (line 273)


isinstance is preferred to type since isinstance keeps track of inheritance.



src/main/python/apache/aurora/admin/admin_util.py (line 276)


is there a test for this switch?



src/test/python/apache/aurora/admin/test_admin.py (line 232)


is there a mock with bypass_leader_redirect = True?


- Dmitriy Shirchenko


On Sept. 20, 2016, 5:32 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52087/
> ---
> 
> (Updated Sept. 20, 2016, 5:32 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fix host maintenance commands to properly initialize the api client.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/admin.py 
> 9fc89a2842d4651ac10aa82118531c450f051712 
>   src/main/python/apache/aurora/admin/admin_util.py 
> 394deb57af9ad8832a02ceab15f33b3c1e5c902b 
>   src/main/python/apache/aurora/admin/host_maintenance.py 
> 677f870c50d1e19d34de36a3c677bb1dcd255ba0 
>   src/main/python/apache/aurora/admin/maintenance.py 
> bf446515222a1246b7fe32acd67315f8145a1212 
>   src/test/python/apache/aurora/admin/test_admin.py 
> f720742c50f774b9f5a41ba57ef251d08d79e8d1 
>   src/test/python/apache/aurora/admin/test_admin_sla.py 
> 54b5a823903b82c2082353ade132eb7b63e518db 
>   src/test/python/apache/aurora/admin/util.py 
> d0a915cd8edee6b36d671b41f43a3af4fe751ae7 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
> e36726e0c0154e51bba40704e13eebec5c0f0d65 
> 
> Diff: https://reviews.apache.org/r/52087/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ran e2e tests.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 52087: Fix host maintenance commands to properly initialize the api client.

[Maxim Khutornenko]

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


Ship it!




Ship It!

- Maxim Khutornenko


On Sept. 20, 2016, 5:32 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52087/
> ---
> 
> (Updated Sept. 20, 2016, 5:32 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fix host maintenance commands to properly initialize the api client.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/admin.py 
> 9fc89a2842d4651ac10aa82118531c450f051712 
>   src/main/python/apache/aurora/admin/admin_util.py 
> 394deb57af9ad8832a02ceab15f33b3c1e5c902b 
>   src/main/python/apache/aurora/admin/host_maintenance.py 
> 677f870c50d1e19d34de36a3c677bb1dcd255ba0 
>   src/main/python/apache/aurora/admin/maintenance.py 
> bf446515222a1246b7fe32acd67315f8145a1212 
>   src/test/python/apache/aurora/admin/test_admin.py 
> f720742c50f774b9f5a41ba57ef251d08d79e8d1 
>   src/test/python/apache/aurora/admin/test_admin_sla.py 
> 54b5a823903b82c2082353ade132eb7b63e518db 
>   src/test/python/apache/aurora/admin/util.py 
> d0a915cd8edee6b36d671b41f43a3af4fe751ae7 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
> e36726e0c0154e51bba40704e13eebec5c0f0d65 
> 
> Diff: https://reviews.apache.org/r/52087/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ran e2e tests.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>