Mailing-List: contact reviews-help@aurora.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: reviews@aurora.incubator.apache.org
Content-Type: multipart/alternative;
 boundary="===============0854777412329189015=="
MIME-Version: 1.0
Subject: Re: Review Request 25721: Asynchronous JS for  Scheduler UI
From: "Bill Farner" <wfarner@apache.org>
To: "Bill Farner" <wfarner@apache.org>, "Kevin Sweeney" <kevints@apache.org>,
 "Joshua Cohen" <jcohen@twopensource.com>
Cc: "Aurora" <reviews@aurora.incubator.apache.org>,
 "David McLaughlin" <david@dmclaughlin.com>
Date: Wed, 17 Sep 2014 04:34:07 -0000
Message-ID: <20140917043407.7803.57016@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Bill Farner" <noreply@reviews.apache.org>
References: <20140917004840.7800.82325@reviews.apache.org>
In-Reply-To: <20140917004840.7800.82325@reviews.apache.org>
Reply-To: "Bill Farner" <wfarner@apache.org>

--===============0854777412329189015==
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/25721/#review53651
-----------------------------------------------------------

Ship it!


src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js
<https://reviews.apache.org/r/25721/#comment93371>

    More objective?  I don't want to kill the humor, but an objective statement will be more useful to the next developer.
    
    Also, would it make sense to drop ao TODO to add a spinner or something rather than empty data?  This may be obliterated by an overhaul, but making note that this is suboptimal user experience would be a plus.


- Bill Farner


On Sept. 17, 2014, 12:48 a.m., David McLaughlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25721/
> -----------------------------------------------------------
> 
> (Updated Sept. 17, 2014, 12:48 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner.
> 
> 
> Bugs: AURORA-700
>     https://issues.apache.org/jira/browse/AURORA-700
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Asynchronous JS for Scheduler UI.
> 
> I have tried to change the minimum amount of JavaScript to keep this review small, even though doing this made me really want to tear everything up and start again :-)
> 
> I attached two screenshots to show the sync vs async behaviour in the browser - note that the async version is 2x the latency of the first. This is because the getJobSummary requests is 10KB compared to <1KB in the sync version. The point is how work is done in parallel. 
> 
> 
> Diffs
> -----
> 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/job.html 14dce65158eab83906c68f9afabf49e39283287d 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 0884cc8f0504a953ef694dae0e6b05ba6e2bff61 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js c80146aa3829e3c3102645a1864dbeaf5e2e56bc 
> 
> Diff: https://reviews.apache.org/r/25721/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew jsHint
> 
> I did manual testing to verify I didn't accidentally introduce any regressions. I have around 80% confidence there are no regressions here, mainly because I wasted an hour on totally unintuitive behaviour from SmartTable. So I'm going to do a bunch more testing, which will involve mocks for updates and crons.
> 
> 
> File Attachments
> ----------------
> 
> Before: Synchronous, serial evaluation of network requests.
>   https://reviews.apache.org/media/uploaded/files/2014/09/17/ce60917a-5c25-4600-8c1f-cc816aa96a5e__before-sync.png
> AFTER: Asynchronous, parallel network requests.
>   https://reviews.apache.org/media/uploaded/files/2014/09/17/dc703579-7072-4aa0-b51a-6df99521fcd5__after-async.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>


--===============0854777412329189015==--