Re: Review Request 25450: Fix broken large update warning.

2014-09-09 Thread Mark Chu-Carroll


 On Sept. 8, 2014, 4:31 p.m., Maxim Khutornenko wrote:
  src/main/python/apache/aurora/client/cli/jobs.py, line 655
  https://reviews.apache.org/r/25450/diff/1/?file=682752#file682752line655
 
  This feels redundant. Current output already has success message(s):
  
  ...
  log(info): Killed: 8
  log(info): Instance 0 has been up and healthy for at least 45 seconds
  log(info): Instance 1 has been up and healthy for at least 45 seconds
  log(info): Update successful
  log(info): Command terminated successfully

Current output has those messages on logs - which are optional.  I think that 
making sure there's some feedback to the user is worthwhile.


 On Sept. 8, 2014, 4:31 p.m., Maxim Khutornenko wrote:
  src/test/python/apache/aurora/client/cli/test_update.py, line 251
  https://reviews.apache.org/r/25450/diff/1/?file=682753#file682753line251
 
  This patching not appear to work in the current version of tests where 
  a 5 seconds timeout is always waited on. Any chance it could be fixed 
  before piling up more test cases like this?

I'll look, and see if I can figure out where that's coming from.


- Mark


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


On Sept. 8, 2014, 2:37 p.m., Mark Chu-Carroll wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25450/
 ---
 
 (Updated Sept. 8, 2014, 2:37 p.m.)
 
 
 Review request for Aurora, David McLaughlin and Maxim Khutornenko.
 
 
 Bugs: aurora-5860
 https://issues.apache.org/jira/browse/aurora-5860
 
 
 Repository: aurora
 
 
 Description
 ---
 
 At some point, the large update warning logic was revised so that the
 warning shows whenever you do an update that either:
 - the updated config had more than 4x as many instances as the running one; or
 - the updated config had less than 4x as many instances as the running one.
 
 (Seriously: either local = 4 * remote or local = 4 * remote. And running 
 git blame says it's all my fault.)
 
 This fixes it: the correct logic is:
 - the updated config has more than 4x as many as running; or
 - the updated config has less than 1/4th as many as running.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/jobs.py 
 8f349c09637c16e2499e85f2dc96eb7ccffd0aaf 
   src/test/python/apache/aurora/client/cli/test_update.py 
 8b7d11202b35deb09a248cfe0a96458fb70c 
   src/test/python/apache/aurora/client/cli/util.py 
 95a2123e127c9811fd2305e71cfc5c7c4376f904 
 
 Diff: https://reviews.apache.org/r/25450/diff/
 
 
 Testing
 ---
 
 Added new unit tests.
 
 
 Thanks,
 
 Mark Chu-Carroll
 




Re: Review Request 25450: Fix broken large update warning.

2014-09-09 Thread Mark Chu-Carroll


 On Sept. 8, 2014, 4:31 p.m., Maxim Khutornenko wrote:
  src/main/python/apache/aurora/client/cli/jobs.py, line 655
  https://reviews.apache.org/r/25450/diff/1/?file=682752#file682752line655
 
  This feels redundant. Current output already has success message(s):
  
  ...
  log(info): Killed: 8
  log(info): Instance 0 has been up and healthy for at least 45 seconds
  log(info): Instance 1 has been up and healthy for at least 45 seconds
  log(info): Update successful
  log(info): Command terminated successfully
 
 Mark Chu-Carroll wrote:
 Current output has those messages on logs - which are optional.  I think 
 that making sure there's some feedback to the user is worthwhile.

(For what it's worth, the command terminated successfully was intended to put 
a status line into loggie. I've switched it to TRANSCRIPT level, so it goes to 
loggie, but the user won't see it; that way, the user won't see duplicates, but 
they'll definitely see feedback when the command completes.)


- Mark


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


On Sept. 8, 2014, 2:37 p.m., Mark Chu-Carroll wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25450/
 ---
 
 (Updated Sept. 8, 2014, 2:37 p.m.)
 
 
 Review request for Aurora, David McLaughlin and Maxim Khutornenko.
 
 
 Bugs: aurora-5860
 https://issues.apache.org/jira/browse/aurora-5860
 
 
 Repository: aurora
 
 
 Description
 ---
 
 At some point, the large update warning logic was revised so that the
 warning shows whenever you do an update that either:
 - the updated config had more than 4x as many instances as the running one; or
 - the updated config had less than 4x as many instances as the running one.
 
 (Seriously: either local = 4 * remote or local = 4 * remote. And running 
 git blame says it's all my fault.)
 
 This fixes it: the correct logic is:
 - the updated config has more than 4x as many as running; or
 - the updated config has less than 1/4th as many as running.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/jobs.py 
 8f349c09637c16e2499e85f2dc96eb7ccffd0aaf 
   src/test/python/apache/aurora/client/cli/test_update.py 
 8b7d11202b35deb09a248cfe0a96458fb70c 
   src/test/python/apache/aurora/client/cli/util.py 
 95a2123e127c9811fd2305e71cfc5c7c4376f904 
 
 Diff: https://reviews.apache.org/r/25450/diff/
 
 
 Testing
 ---
 
 Added new unit tests.
 
 
 Thanks,
 
 Mark Chu-Carroll
 




Re: Review Request 25450: Fix broken large update warning.

2014-09-09 Thread Mark Chu-Carroll

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

(Updated Sept. 9, 2014, 3:43 p.m.)


Review request for Aurora, David McLaughlin and Maxim Khutornenko.


Changes
---

Fixed time delay in test runs;
Adjusted logging output.


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


Repository: aurora


Description
---

At some point, the large update warning logic was revised so that the
warning shows whenever you do an update that either:
- the updated config had more than 4x as many instances as the running one; or
- the updated config had less than 4x as many instances as the running one.

(Seriously: either local = 4 * remote or local = 4 * remote. And running git 
blame says it's all my fault.)

This fixes it: the correct logic is:
- the updated config has more than 4x as many as running; or
- the updated config has less than 1/4th as many as running.


Diffs (updated)
-

  src/main/python/apache/aurora/client/cli/__init__.py 
6e553d8af459e575b2d62282a3bc0d1e266203d8 
  src/main/python/apache/aurora/client/cli/jobs.py 
8f349c09637c16e2499e85f2dc96eb7ccffd0aaf 
  src/test/python/apache/aurora/client/cli/test_update.py 
8b7d11202b35deb09a248cfe0a96458fb70c 
  src/test/python/apache/aurora/client/cli/util.py 
95a2123e127c9811fd2305e71cfc5c7c4376f904 

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


Testing
---

Added new unit tests.


Thanks,

Mark Chu-Carroll



Re: Review Request 25450: Fix broken large update warning.

2014-09-09 Thread Maxim Khutornenko

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

Ship it!


Ship It!

- Maxim Khutornenko


On Sept. 9, 2014, 7:43 p.m., Mark Chu-Carroll wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25450/
 ---
 
 (Updated Sept. 9, 2014, 7:43 p.m.)
 
 
 Review request for Aurora, David McLaughlin and Maxim Khutornenko.
 
 
 Bugs: aurora-5860
 https://issues.apache.org/jira/browse/aurora-5860
 
 
 Repository: aurora
 
 
 Description
 ---
 
 At some point, the large update warning logic was revised so that the
 warning shows whenever you do an update that either:
 - the updated config had more than 4x as many instances as the running one; or
 - the updated config had less than 4x as many instances as the running one.
 
 (Seriously: either local = 4 * remote or local = 4 * remote. And running 
 git blame says it's all my fault.)
 
 This fixes it: the correct logic is:
 - the updated config has more than 4x as many as running; or
 - the updated config has less than 1/4th as many as running.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/__init__.py 
 6e553d8af459e575b2d62282a3bc0d1e266203d8 
   src/main/python/apache/aurora/client/cli/jobs.py 
 8f349c09637c16e2499e85f2dc96eb7ccffd0aaf 
   src/test/python/apache/aurora/client/cli/test_update.py 
 8b7d11202b35deb09a248cfe0a96458fb70c 
   src/test/python/apache/aurora/client/cli/util.py 
 95a2123e127c9811fd2305e71cfc5c7c4376f904 
 
 Diff: https://reviews.apache.org/r/25450/diff/
 
 
 Testing
 ---
 
 Added new unit tests.
 
 
 Thanks,
 
 Mark Chu-Carroll
 




Re: Review Request 25450: Fix broken large update warning.

2014-09-09 Thread David McLaughlin

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

Ship it!


Ship It!

- David McLaughlin


On Sept. 9, 2014, 7:43 p.m., Mark Chu-Carroll wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25450/
 ---
 
 (Updated Sept. 9, 2014, 7:43 p.m.)
 
 
 Review request for Aurora, David McLaughlin and Maxim Khutornenko.
 
 
 Bugs: aurora-5860
 https://issues.apache.org/jira/browse/aurora-5860
 
 
 Repository: aurora
 
 
 Description
 ---
 
 At some point, the large update warning logic was revised so that the
 warning shows whenever you do an update that either:
 - the updated config had more than 4x as many instances as the running one; or
 - the updated config had less than 4x as many instances as the running one.
 
 (Seriously: either local = 4 * remote or local = 4 * remote. And running 
 git blame says it's all my fault.)
 
 This fixes it: the correct logic is:
 - the updated config has more than 4x as many as running; or
 - the updated config has less than 1/4th as many as running.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/__init__.py 
 6e553d8af459e575b2d62282a3bc0d1e266203d8 
   src/main/python/apache/aurora/client/cli/jobs.py 
 8f349c09637c16e2499e85f2dc96eb7ccffd0aaf 
   src/test/python/apache/aurora/client/cli/test_update.py 
 8b7d11202b35deb09a248cfe0a96458fb70c 
   src/test/python/apache/aurora/client/cli/util.py 
 95a2123e127c9811fd2305e71cfc5c7c4376f904 
 
 Diff: https://reviews.apache.org/r/25450/diff/
 
 
 Testing
 ---
 
 Added new unit tests.
 
 
 Thanks,
 
 Mark Chu-Carroll
 




Review Request 25450: Fix broken large update warning.

2014-09-08 Thread Mark Chu-Carroll

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

Review request for Aurora, David McLaughlin and Maxim Khutornenko.


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


Repository: aurora


Description
---

At some point, the large update warning logic was revised so that the
warning shows whenever you do an update that either:
- the updated config had more than 4x as many instances as the running one; or
- the updated config had less than 4x as many instances as the running one.

(Seriously: either local = 4 * remote or local = 4 * remote. And running git 
blame says it's all my fault.)

This fixes it: the correct logic is:
- the updated config has more than 4x as many as running; or
- the updated config has less than 1/4th as many as running.


Diffs
-

  src/main/python/apache/aurora/client/cli/jobs.py 
8f349c09637c16e2499e85f2dc96eb7ccffd0aaf 
  src/test/python/apache/aurora/client/cli/test_update.py 
8b7d11202b35deb09a248cfe0a96458fb70c 
  src/test/python/apache/aurora/client/cli/util.py 
95a2123e127c9811fd2305e71cfc5c7c4376f904 

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


Testing
---

Added new unit tests.


Thanks,

Mark Chu-Carroll



Re: Review Request 25450: Fix broken large update warning.

2014-09-08 Thread Maxim Khutornenko

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



src/main/python/apache/aurora/client/cli/jobs.py
https://reviews.apache.org/r/25450/#comment91477

This feels redundant. Current output already has success message(s):

...
log(info): Killed: 8
log(info): Instance 0 has been up and healthy for at least 45 seconds
log(info): Instance 1 has been up and healthy for at least 45 seconds
log(info): Update successful
log(info): Command terminated successfully



src/test/python/apache/aurora/client/cli/test_update.py
https://reviews.apache.org/r/25450/#comment91480

This patching not appear to work in the current version of tests where a 5 
seconds timeout is always waited on. Any chance it could be fixed before piling 
up more test cases like this?


- Maxim Khutornenko


On Sept. 8, 2014, 6:37 p.m., Mark Chu-Carroll wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25450/
 ---
 
 (Updated Sept. 8, 2014, 6:37 p.m.)
 
 
 Review request for Aurora, David McLaughlin and Maxim Khutornenko.
 
 
 Bugs: aurora-5860
 https://issues.apache.org/jira/browse/aurora-5860
 
 
 Repository: aurora
 
 
 Description
 ---
 
 At some point, the large update warning logic was revised so that the
 warning shows whenever you do an update that either:
 - the updated config had more than 4x as many instances as the running one; or
 - the updated config had less than 4x as many instances as the running one.
 
 (Seriously: either local = 4 * remote or local = 4 * remote. And running 
 git blame says it's all my fault.)
 
 This fixes it: the correct logic is:
 - the updated config has more than 4x as many as running; or
 - the updated config has less than 1/4th as many as running.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/jobs.py 
 8f349c09637c16e2499e85f2dc96eb7ccffd0aaf 
   src/test/python/apache/aurora/client/cli/test_update.py 
 8b7d11202b35deb09a248cfe0a96458fb70c 
   src/test/python/apache/aurora/client/cli/util.py 
 95a2123e127c9811fd2305e71cfc5c7c4376f904 
 
 Diff: https://reviews.apache.org/r/25450/diff/
 
 
 Testing
 ---
 
 Added new unit tests.
 
 
 Thanks,
 
 Mark Chu-Carroll