Re: Review Request 34373: Exposing task reconciliation stats.

2015-05-19 Thread Maxim Khutornenko


 On May 19, 2015, 9:46 p.m., Ben Mahler wrote:
  src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java, lines 
  214-217
  https://reviews.apache.org/r/34373/diff/2/?file=963238#file963238line214
 
  Any reason not to pass the '`reason`' directly, rather than passing the 
  whole `TaskStatus`?

Just making the helper method more useful by offloading all name extracting 
logic into it making the consumer code cleaner.


- Maxim


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


On May 18, 2015, 10:42 p.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34373/
 ---
 
 (Updated May 18, 2015, 10:42 p.m.)
 
 
 Review request for Aurora, Ben Mahler and Zameer Manji.
 
 
 Bugs: AURORA-1047
 https://issues.apache.org/jira/browse/AURORA-1047
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Adding counters for the new `StatusChangeResult` values.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 
 f1e5dd2d47af2570f404d198b3fbdd754174206e 
   src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 
 f4631c1cec4386d813d5869c28289207acf8523f 
 
 Diff: https://reviews.apache.org/r/34373/diff/
 
 
 Testing
 ---
 
 ./gradlew -Pq build
 
 
 Thanks,
 
 Maxim Khutornenko
 




Re: Review Request 34373: Exposing task reconciliation stats.

2015-05-19 Thread Ben Mahler

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


Looks good to me. :)


src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java
https://reviews.apache.org/r/34373/#comment135623

Any reason not to pass the '`reason`' directly, rather than passing the 
whole `TaskStatus`?


- Ben Mahler


On May 18, 2015, 10:42 p.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34373/
 ---
 
 (Updated May 18, 2015, 10:42 p.m.)
 
 
 Review request for Aurora, Ben Mahler and Zameer Manji.
 
 
 Bugs: AURORA-1047
 https://issues.apache.org/jira/browse/AURORA-1047
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Adding counters for the new `StatusChangeResult` values.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 
 f1e5dd2d47af2570f404d198b3fbdd754174206e 
   src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 
 f4631c1cec4386d813d5869c28289207acf8523f 
 
 Diff: https://reviews.apache.org/r/34373/diff/
 
 
 Testing
 ---
 
 ./gradlew -Pq build
 
 
 Thanks,
 
 Maxim Khutornenko
 




Re: Review Request 34373: Exposing task reconciliation stats.

2015-05-18 Thread Aurora ReviewBot

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

Ship it!


Master (920263b) 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 May 18, 2015, 9:41 p.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34373/
 ---
 
 (Updated May 18, 2015, 9:41 p.m.)
 
 
 Review request for Aurora, Ben Mahler and Zameer Manji.
 
 
 Bugs: AURORA-1047
 https://issues.apache.org/jira/browse/AURORA-1047
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Adding counters for the new `StatusChangeResult` values.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 
 f1e5dd2d47af2570f404d198b3fbdd754174206e 
   src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 
 f4631c1cec4386d813d5869c28289207acf8523f 
 
 Diff: https://reviews.apache.org/r/34373/diff/
 
 
 Testing
 ---
 
 ./gradlew -Pq build
 
 
 Thanks,
 
 Maxim Khutornenko
 




Re: Review Request 34373: Exposing task reconciliation stats.

2015-05-18 Thread Maxim Khutornenko


 On May 18, 2015, 10:05 p.m., Zameer Manji wrote:
  src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java, line 204
  https://reviews.apache.org/r/34373/diff/1/?file=963203#file963203line204
 
  Special-casing the reconsiliation reason and not the others smells like 
  trouble to me. Why can't we break out stats for all of the reasons?

Thanks, had a similar thought but decided to skip as we already expose reason 
counters (task_exit_*). It certainly does not hurt to know the whole 
reason/result matrix. Done.


- Maxim


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


On May 18, 2015, 9:41 p.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34373/
 ---
 
 (Updated May 18, 2015, 9:41 p.m.)
 
 
 Review request for Aurora, Ben Mahler and Zameer Manji.
 
 
 Bugs: AURORA-1047
 https://issues.apache.org/jira/browse/AURORA-1047
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Adding counters for the new `StatusChangeResult` values.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 
 f1e5dd2d47af2570f404d198b3fbdd754174206e 
   src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 
 f4631c1cec4386d813d5869c28289207acf8523f 
 
 Diff: https://reviews.apache.org/r/34373/diff/
 
 
 Testing
 ---
 
 ./gradlew -Pq build
 
 
 Thanks,
 
 Maxim Khutornenko
 




Re: Review Request 34373: Exposing task reconciliation stats.

2015-05-18 Thread Maxim Khutornenko

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

(Updated May 18, 2015, 10:42 p.m.)


Review request for Aurora, Ben Mahler and Zameer Manji.


Changes
---

Zameer's comments.


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


Repository: aurora


Description
---

Adding counters for the new `StatusChangeResult` values.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 
f1e5dd2d47af2570f404d198b3fbdd754174206e 
  src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 
f4631c1cec4386d813d5869c28289207acf8523f 

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


Testing
---

./gradlew -Pq build


Thanks,

Maxim Khutornenko



Re: Review Request 34373: Exposing task reconciliation stats.

2015-05-18 Thread Aurora ReviewBot

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

Ship it!


Master (920263b) 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 May 18, 2015, 10:42 p.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34373/
 ---
 
 (Updated May 18, 2015, 10:42 p.m.)
 
 
 Review request for Aurora, Ben Mahler and Zameer Manji.
 
 
 Bugs: AURORA-1047
 https://issues.apache.org/jira/browse/AURORA-1047
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Adding counters for the new `StatusChangeResult` values.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 
 f1e5dd2d47af2570f404d198b3fbdd754174206e 
   src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 
 f4631c1cec4386d813d5869c28289207acf8523f 
 
 Diff: https://reviews.apache.org/r/34373/diff/
 
 
 Testing
 ---
 
 ./gradlew -Pq build
 
 
 Thanks,
 
 Maxim Khutornenko