Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id D59FB200BD1 for ; Mon, 28 Nov 2016 17:04:01 +0100 (CET) Received: by cust-asf.ponee.io (Postfix) id D4492160B22; Mon, 28 Nov 2016 16:04:01 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 27293160B00 for ; Mon, 28 Nov 2016 17:04:01 +0100 (CET) Received: (qmail 99937 invoked by uid 500); 28 Nov 2016 16:04:00 -0000 Mailing-List: contact reviews-help@aurora.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: reviews@aurora.apache.org Delivered-To: mailing list reviews@aurora.apache.org Received: (qmail 99911 invoked by uid 99); 28 Nov 2016 16:04:00 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 28 Nov 2016 16:04:00 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id 2F0F72F6B57; Mon, 28 Nov 2016 16:03:58 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============1970531995052448124==" MIME-Version: 1.0 Subject: Re: Review Request 51993: Added the 'reason' to the /pendingTasks endpoint From: Joshua Cohen To: Joshua Cohen Cc: Aurora , Pradyumna Kaushik Date: Mon, 28 Nov 2016 16:03:58 -0000 Message-ID: <20161128160358.5575.39029@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org/ Auto-Submitted: auto-generated Sender: Joshua Cohen X-ReviewGroup: Aurora X-Auto-Response-Suppress: DR, RN, OOF, AutoReply X-ReviewRequest-URL: https://reviews.apache.org/r/51993/ X-Sender: Joshua Cohen References: <20161103224637.1746.97407@reviews.apache.org> In-Reply-To: <20161103224637.1746.97407@reviews.apache.org> X-ReviewBoard-Diff-For: src/test/java/org/apache/aurora/scheduler/http/PendingTasksTest.java X-ReviewBoard-Diff-For: src/test/java/org/apache/aurora/scheduler/http/TestUtils.java Reply-To: Joshua Cohen X-ReviewRequest-Repository: aurora archived-at: Mon, 28 Nov 2016 16:04:02 -0000 --===============1970531995052448124== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit ----------------------------------------------------------- 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) Indentation is off, should just be 4 spaces. src/main/java/org/apache/aurora/scheduler/http/PendingTasks.java (lines 70 - 71) This fits on one line. src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java (lines 132 - 133) Also fits on one line. src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java (line 138) 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) 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 > > --===============1970531995052448124==--