You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Joshua Cohen <jc...@apache.org> on 2016/11/28 16:03:58 UTC

Re: Review Request 51993: Added the 'reason' to the /pendingTasks endpoint

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



Sorry for the delay on this, everything looks good to me, just a few style/maintainability nits, then it's good to go!


src/main/java/org/apache/aurora/scheduler/http/PendingTasks.java (line 60)
<https://reviews.apache.org/r/51993/#comment227419>

    Indentation is off, should just be 4 spaces.



src/main/java/org/apache/aurora/scheduler/http/PendingTasks.java (lines 70 - 71)
<https://reviews.apache.org/r/51993/#comment227421>

    This fits on one line.



src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java (lines 132 - 133)
<https://reviews.apache.org/r/51993/#comment227424>

    Also fits on one line.



src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java (line 138)
<https://reviews.apache.org/r/51993/#comment227425>

    nit: Use `GuavaUtils#toImmutableMap` here so we return an immutable map.
    
    Technically should do the same for the list of reasons within the stream, but there's no chance of that leaking, so it's probably fine as is.



src/main/java/org/apache/aurora/scheduler/scheduling/TaskGroup.java (line 45)
<https://reviews.apache.org/r/51993/#comment227426>

    Can you add a comment explaining that this class is serialized by the `PendingTasks` endpoint, but the key is exposed below via the `getName` method?


- Joshua Cohen


On Nov. 3, 2016, 10:46 p.m., Pradyumna Kaushik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51993/
> -----------------------------------------------------------
> 
> (Updated Nov. 3, 2016, 10:46 p.m.)
> 
> 
> Review request for Aurora and Joshua Cohen.
> 
> 
> Bugs: AURORA-1762
>     https://issues.apache.org/jira/browse/AURORA-1762
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Added the 'reason' to the /pendingTasks endpoint
> 
> 
> Diffs
> -----
> 
>   config/legacy_untested_classes.txt 84265066001dee79df6bc16de6de9a165e912b9b 
>   src/main/java/org/apache/aurora/scheduler/http/PendingTasks.java c80e0c8adf80e12082a6952ae79b7d9cc960c5b6 
>   src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java f783e7ff220573915524a1efc27141193d19fa6c 
>   src/main/java/org/apache/aurora/scheduler/scheduling/TaskGroup.java b521620badff76e8fab0bdf31f8a73c4019b2121 
>   src/main/java/org/apache/aurora/scheduler/scheduling/TaskGroups.java 77187bc19fb1f783d1b820d324c7fe18e509365f 
>   src/test/java/org/apache/aurora/scheduler/http/OffersTest.java 9e3573252cf37153180b1fc5ab9150bab0299c99 
>   src/test/java/org/apache/aurora/scheduler/http/PendingTasksTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/http/TestUtils.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java d9b3cc672f42c50b2a2a142733d26c0725bbc864 
> 
> Diff: https://reviews.apache.org/r/51993/diff/
> 
> 
> Testing
> -------
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Pradyumna Kaushik
> 
>


Re: Review Request 51993: Added the 'reason' to the /pendingTasks endpoint

Posted by Pradyumna Kaushik <pk...@binghamton.edu>.

> On Nov. 28, 2016, 4:03 p.m., Joshua Cohen wrote:
> > Sorry for the delay on this, everything looks good to me, just a few style/maintainability nits, then it's good to go!

@jcohen. I have made the necessary changes and have posted the patch for review. Thank you.


- Pradyumna


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


On Nov. 28, 2016, 8:34 p.m., Pradyumna Kaushik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51993/
> -----------------------------------------------------------
> 
> (Updated Nov. 28, 2016, 8:34 p.m.)
> 
> 
> Review request for Aurora and Joshua Cohen.
> 
> 
> Bugs: AURORA-1762
>     https://issues.apache.org/jira/browse/AURORA-1762
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Added the 'reason' to the /pendingTasks endpoint
> 
> 
> Diffs
> -----
> 
>   config/legacy_untested_classes.txt 84265066001dee79df6bc16de6de9a165e912b9b 
>   src/main/java/org/apache/aurora/scheduler/http/PendingTasks.java c80e0c8adf80e12082a6952ae79b7d9cc960c5b6 
>   src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java f783e7ff220573915524a1efc27141193d19fa6c 
>   src/main/java/org/apache/aurora/scheduler/scheduling/TaskGroup.java b521620badff76e8fab0bdf31f8a73c4019b2121 
>   src/main/java/org/apache/aurora/scheduler/scheduling/TaskGroups.java 77187bc19fb1f783d1b820d324c7fe18e509365f 
>   src/test/java/org/apache/aurora/scheduler/http/OffersTest.java 9e3573252cf37153180b1fc5ab9150bab0299c99 
>   src/test/java/org/apache/aurora/scheduler/http/PendingTasksTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/http/TestUtils.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java d9b3cc672f42c50b2a2a142733d26c0725bbc864 
> 
> Diff: https://reviews.apache.org/r/51993/diff/
> 
> 
> Testing
> -------
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Pradyumna Kaushik
> 
>