You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by David McLaughlin <da...@dmclaughlin.com> on 2014/09/17 02:48:40 UTC
Review Request 25721: Asynchronous JS for Scheduler UI
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25721/
-----------------------------------------------------------
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
Re: Review Request 25721: Asynchronous JS for Scheduler UI
Posted by David McLaughlin <da...@dmclaughlin.com>.
> On Sept. 17, 2014, 4:34 a.m., Bill Farner wrote:
> > src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js, line 368
> > <https://reviews.apache.org/r/25721/diff/1/?file=691307#file691307line368>
> >
> > 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.
I added a little "loading job information" div while the tasks query executes, because that's the main thing that might confuse users. Filed AURORA-721 for solving this for all network requests.
- David
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25721/#review53651
-----------------------------------------------------------
On Sept. 17, 2014, 5:49 p.m., David McLaughlin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25721/
> -----------------------------------------------------------
>
> (Updated Sept. 17, 2014, 5:49 p.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
>
>
Re: Review Request 25721: Asynchronous JS for Scheduler UI
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
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
>
>
Re: Review Request 25721: Asynchronous JS for Scheduler UI
Posted by David McLaughlin <da...@dmclaughlin.com>.
> On Sept. 17, 2014, 12:52 a.m., Bill Farner wrote:
> > > This is because the getJobSummary requests is 10KB compared to <1KB in the sync version.
> >
> > I didn't grok this part. The _request_ is 10 KB? Nonetheless, why does sync/async change data representation in either direction?
I took the screenshot on master before I started working on this and my hello_word job had 1 active task and 0 complete tasks. I left that flapping task running while I worked on async, which generated an extra 100 bad completed tasks which obviously increased the size of the response.
- David
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25721/#review53630
-----------------------------------------------------------
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
>
>
Re: Review Request 25721: Asynchronous JS for Scheduler UI
Posted by Bill Farner <wf...@apache.org>.
> On Sept. 17, 2014, 12:52 a.m., Bill Farner wrote:
> > > This is because the getJobSummary requests is 10KB compared to <1KB in the sync version.
> >
> > I didn't grok this part. The _request_ is 10 KB? Nonetheless, why does sync/async change data representation in either direction?
>
> David McLaughlin wrote:
> I took the screenshot on master before I started working on this and my hello_word job had 1 active task and 0 complete tasks. I left that flapping task running while I worked on async, which generated an extra 100 bad completed tasks which obviously increased the size of the response.
Oh i see, these requests/responses were just plain different. Thanks!
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25721/#review53630
-----------------------------------------------------------
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
>
>
Re: Review Request 25721: Asynchronous JS for Scheduler UI
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25721/#review53630
-----------------------------------------------------------
> This is because the getJobSummary requests is 10KB compared to <1KB in the sync version.
I didn't grok this part. The _request_ is 10 KB? Nonetheless, why does sync/async change data representation in either direction?
- 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
>
>
Re: Review Request 25721: Asynchronous JS for Scheduler UI
Posted by Joshua Cohen <jc...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25721/#review53733
-----------------------------------------------------------
Ship it!
Ship It!
- Joshua Cohen
On Sept. 17, 2014, 5:49 p.m., David McLaughlin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25721/
> -----------------------------------------------------------
>
> (Updated Sept. 17, 2014, 5:49 p.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
>
>
Re: Review Request 25721: Asynchronous JS for Scheduler UI
Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25721/#review53874
-----------------------------------------------------------
Ship it!
Ship It!
- Kevin Sweeney
On Sept. 18, 2014, 2:29 p.m., David McLaughlin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25721/
> -----------------------------------------------------------
>
> (Updated Sept. 18, 2014, 2:29 p.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 a454b2670d2362a8c89e339b8574989057c9d5d4
> src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 7d9c646c1d9e6d40aff9f23c725cf52f56c07203
>
> 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
>
>
Re: Review Request 25721: Asynchronous JS for Scheduler UI
Posted by David McLaughlin <da...@dmclaughlin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25721/
-----------------------------------------------------------
(Updated Sept. 18, 2014, 9:29 p.m.)
Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner.
Changes
-------
Rebase + local testing to increae confidence this is good to ship (which caught a somewhat unrelated bug).
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 (updated)
-----
src/main/resources/org/apache/aurora/scheduler/http/ui/job.html 14dce65158eab83906c68f9afabf49e39283287d
src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js a454b2670d2362a8c89e339b8574989057c9d5d4
src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 7d9c646c1d9e6d40aff9f23c725cf52f56c07203
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
Re: Review Request 25721: Asynchronous JS for Scheduler UI
Posted by David McLaughlin <da...@dmclaughlin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25721/
-----------------------------------------------------------
(Updated Sept. 17, 2014, 5:49 p.m.)
Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner.
Changes
-------
RB feedback.
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 (updated)
-----
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