Re: Review Request 24720: Expand actions in JobUpdateAction
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24720/ --- (Updated Aug. 15, 2014, 5:52 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Repository: aurora Description (updated) --- Added some more items to the JobUpdateAction enum. Not married to the labels I've chosen, and I wasn't sure if we want to break out INSTANCE_UPDATING into finer-grained actions. Expected actions: 1) instance update: INSTANCE_UPDATED 2) instance remove: INSTANCE_REMOVED 3) instance add: INSTANCE_ADDED -> INSTANCE_UPDATING -> INSTANCE_UPDATED 4) noop: INSTANCE_SKIPPED 5) instance rolled back: INSTANCE_ROLLED_BACK Diffs (updated) - src/main/thrift/org/apache/aurora/gen/api.thrift af9f02ed1de487bc5cc2967d2edcece5b21e0be5 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java f695b85514bcc5cedb16e962124af3db052cb17a src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java ebcb9103d75909080f5b6a69db3a1bf46cfd9780 src/test/resources/org/apache/aurora/gen/api.thrift.md5 21a05f6939da1dd7fc15cf6336bc3fee283f16ab Diff: https://reviews.apache.org/r/24720/diff/ Testing (updated) --- ./gradlew -Pq build Thanks, David McLaughlin
Re: Review Request 24720: Expand actions in JobUpdateAction
> On Aug. 15, 2014, 6:02 p.m., Maxim Khutornenko wrote: > > src/main/thrift/org/apache/aurora/gen/api.thrift, line 558 > > <https://reviews.apache.org/r/24720/diff/2/?file=661560#file661560line558> > > > > Rolling back assumes removing new (INSTANCE_REMOVED) and adding old > > (INSTANCE_ADDED). Do we want to distinct those actions from the forward > > roll? INSTANCE_REMOVED is only used when you remove an instance because the new instanceCount is less than previous one, it's not used if you're removing an instance as part of updating it. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24720/#review50752 ------- On Aug. 15, 2014, 5:52 p.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24720/ > --- > > (Updated Aug. 15, 2014, 5:52 p.m.) > > > Review request for Aurora, Maxim Khutornenko and Bill Farner. > > > Repository: aurora > > > Description > --- > > Added some more items to the JobUpdateAction enum. Not married to the labels > I've chosen, and I wasn't sure if we want to break out INSTANCE_UPDATING into > finer-grained actions. > > > Expected actions: > > 1) instance update: INSTANCE_UPDATED > 2) instance remove: INSTANCE_REMOVED > 3) instance add: INSTANCE_ADDED -> INSTANCE_UPDATING -> INSTANCE_UPDATED > 4) noop: INSTANCE_SKIPPED > 5) instance rolled back: INSTANCE_ROLLED_BACK > > > Diffs > - > > src/main/thrift/org/apache/aurora/gen/api.thrift > af9f02ed1de487bc5cc2967d2edcece5b21e0be5 > > src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java > f695b85514bcc5cedb16e962124af3db052cb17a > src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java > ebcb9103d75909080f5b6a69db3a1bf46cfd9780 > src/test/resources/org/apache/aurora/gen/api.thrift.md5 > 21a05f6939da1dd7fc15cf6336bc3fee283f16ab > > Diff: https://reviews.apache.org/r/24720/diff/ > > > Testing > --- > > ./gradlew -Pq build > > > Thanks, > > David McLaughlin > >
Re: Review Request 24720: Expand actions in JobUpdateAction
> On Aug. 15, 2014, 6:02 p.m., Maxim Khutornenko wrote: > > src/main/thrift/org/apache/aurora/gen/api.thrift, line 558 > > <https://reviews.apache.org/r/24720/diff/2/?file=661560#file661560line558> > > > > Rolling back assumes removing new (INSTANCE_REMOVED) and adding old > > (INSTANCE_ADDED). Do we want to distinct those actions from the forward > > roll? > > David McLaughlin wrote: > INSTANCE_REMOVED is only used when you remove an instance because the new > instanceCount is less than previous one, it's not used if you're removing an > instance as part of updating it. > > Maxim Khutornenko wrote: > I guess I did not see the JobUpdateAction as a post factum update story > teller. I thought it would be used more as a facilitator for internal actions > to be done (i.e. add_instance, kill_instance, and etc.) > > I don't have a problem with this high level mapping as long as updater > would not rely on it to drive the state machine progress. We should be careful either way not to leak internal state machines into the public API. But yes, I think definitely the purpose of instance events is to retell a story of what happened during this update. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24720/#review50752 --- On Aug. 15, 2014, 5:52 p.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24720/ > --- > > (Updated Aug. 15, 2014, 5:52 p.m.) > > > Review request for Aurora, Maxim Khutornenko and Bill Farner. > > > Repository: aurora > > > Description > --- > > Added some more items to the JobUpdateAction enum. Not married to the labels > I've chosen, and I wasn't sure if we want to break out INSTANCE_UPDATING into > finer-grained actions. > > > Expected actions: > > 1) instance update: INSTANCE_UPDATED > 2) instance remove: INSTANCE_REMOVED > 3) instance add: INSTANCE_ADDED -> INSTANCE_UPDATING -> INSTANCE_UPDATED > 4) noop: INSTANCE_SKIPPED > 5) instance rolled back: INSTANCE_ROLLED_BACK > > > Diffs > - > > src/main/thrift/org/apache/aurora/gen/api.thrift > af9f02ed1de487bc5cc2967d2edcece5b21e0be5 > > src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java > f695b85514bcc5cedb16e962124af3db052cb17a > src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java > ebcb9103d75909080f5b6a69db3a1bf46cfd9780 > src/test/resources/org/apache/aurora/gen/api.thrift.md5 > 21a05f6939da1dd7fc15cf6336bc3fee283f16ab > > Diff: https://reviews.apache.org/r/24720/diff/ > > > Testing > --- > > ./gradlew -Pq build > > > Thanks, > > David McLaughlin > >
Re: Review Request 24720: Expand actions in JobUpdateAction
> On Aug. 16, 2014, 12:23 a.m., Maxim Khutornenko wrote: > > src/main/thrift/org/apache/aurora/gen/api.thrift, line 538 > > <https://reviews.apache.org/r/24720/diff/2/?file=661560#file661560line538> > > > > What action is going to be used for an instance that failed to update > > and the job rollback is disabled? Sounds like we need another state for this. Do you have a suggestion? INSTANCE_UPDATE_FAILED ? - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24720/#review50805 --- On Aug. 15, 2014, 5:52 p.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24720/ > --- > > (Updated Aug. 15, 2014, 5:52 p.m.) > > > Review request for Aurora, Maxim Khutornenko and Bill Farner. > > > Repository: aurora > > > Description > --- > > Added some more items to the JobUpdateAction enum. Not married to the labels > I've chosen, and I wasn't sure if we want to break out INSTANCE_UPDATING into > finer-grained actions. > > > Expected actions: > > 1) instance update: INSTANCE_UPDATED > 2) instance remove: INSTANCE_REMOVED > 3) instance add: INSTANCE_ADDED -> INSTANCE_UPDATING -> INSTANCE_UPDATED > 4) noop: INSTANCE_SKIPPED > 5) instance rolled back: INSTANCE_ROLLED_BACK > > > Diffs > - > > src/main/thrift/org/apache/aurora/gen/api.thrift > af9f02ed1de487bc5cc2967d2edcece5b21e0be5 > > src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java > f695b85514bcc5cedb16e962124af3db052cb17a > src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java > ebcb9103d75909080f5b6a69db3a1bf46cfd9780 > src/test/resources/org/apache/aurora/gen/api.thrift.md5 > 21a05f6939da1dd7fc15cf6336bc3fee283f16ab > > Diff: https://reviews.apache.org/r/24720/diff/ > > > Testing > --- > > ./gradlew -Pq build > > > Thanks, > > David McLaughlin > >
Re: Review Request 24720: Expand actions in JobUpdateAction
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24720/ --- (Updated Aug. 18, 2014, 5:49 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Changes --- Failure states. Repository: aurora Description --- Added some more items to the JobUpdateAction enum. Not married to the labels I've chosen, and I wasn't sure if we want to break out INSTANCE_UPDATING into finer-grained actions. Expected actions: 1) instance update: INSTANCE_UPDATED 2) instance remove: INSTANCE_REMOVED 3) instance add: INSTANCE_ADDED -> INSTANCE_UPDATING -> INSTANCE_UPDATED 4) noop: INSTANCE_SKIPPED 5) instance rolled back: INSTANCE_ROLLED_BACK Diffs (updated) - src/main/thrift/org/apache/aurora/gen/api.thrift af9f02ed1de487bc5cc2967d2edcece5b21e0be5 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java f695b85514bcc5cedb16e962124af3db052cb17a src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java ebcb9103d75909080f5b6a69db3a1bf46cfd9780 src/test/resources/org/apache/aurora/gen/api.thrift.md5 21a05f6939da1dd7fc15cf6336bc3fee283f16ab Diff: https://reviews.apache.org/r/24720/diff/ Testing --- ./gradlew -Pq build Thanks, David McLaughlin
Re: Review Request 24720: Expand actions in JobUpdateAction
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24720/ --- (Updated Aug. 18, 2014, 5:59 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Changes --- rebase. (also, a reminder that I need someone to commit this for me.) Repository: aurora Description --- Added some more items to the JobUpdateAction enum. Not married to the labels I've chosen, and I wasn't sure if we want to break out INSTANCE_UPDATING into finer-grained actions. Expected actions: 1) instance update: INSTANCE_UPDATED 2) instance remove: INSTANCE_REMOVED 3) instance add: INSTANCE_ADDED -> INSTANCE_UPDATING -> INSTANCE_UPDATED 4) noop: INSTANCE_SKIPPED 5) instance rolled back: INSTANCE_ROLLED_BACK Diffs (updated) - src/main/thrift/org/apache/aurora/gen/api.thrift af9f02ed1de487bc5cc2967d2edcece5b21e0be5 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java f669dbe472ea2171b0ac898b71626f0b25c0a9ec src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 78798f29bfbcf04b48953d050e498b3cad2248fb src/test/resources/org/apache/aurora/gen/api.thrift.md5 21a05f6939da1dd7fc15cf6336bc3fee283f16ab src/test/resources/org/apache/aurora/gen/storage.thrift.md5 45762990d33969bedde7340887cde16a535e99fe Diff: https://reviews.apache.org/r/24720/diff/ Testing --- ./gradlew -Pq build Thanks, David McLaughlin
Re: Review Request 24852: Add command output tests for "job create", "job killall", "job kill"
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24852/#review50994 --- Ship it! Ship It! - David McLaughlin On Aug. 19, 2014, 3:22 p.m., Mark Chu-Carroll wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24852/ > --- > > (Updated Aug. 19, 2014, 3:22 p.m.) > > > Review request for Aurora, David McLaughlin and Maxim Khutornenko. > > > Bugs: aurora-645 > https://issues.apache.org/jira/browse/aurora-645 > > > Repository: aurora > > > Description > --- > > Add tests to ensure that IO behavior of client commands is correct. > > Improve IO behavior in the IO client, so that users get more information. > Formorely, many commands succeeded silently, when it would be more helpful to > actually tell the user that you did something. (This shouldn't affect > scripting, because in these commands, the only result that's needed by a > scripter is succeess/failure, which is carried by the exit code.) > > > Diffs > - > > src/main/python/apache/aurora/client/cli/context.py > 0e10589c2e8ab6decedb12122a0d8c7a81285e18 > src/main/python/apache/aurora/client/cli/jobs.py > 109ce59e3e8dc1c6f4fb918522718cc47867020d > src/test/python/apache/aurora/client/cli/test_create.py > ca635bd49acec9596db67839451101a59ba4d761 > src/test/python/apache/aurora/client/cli/test_kill.py > ee64908855a4960f44ce96c810e69dd105d2ce5d > src/test/python/apache/aurora/client/cli/test_restart.py > 14a08e873058374a9bfe4ffbff8ae7279123f264 > > Diff: https://reviews.apache.org/r/24852/diff/ > > > Testing > --- > > Ran test suite. > > > Thanks, > > Mark Chu-Carroll > >
Re: Review Request 24953: Fix bug in handling of pystachio bindings.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24953/#review51226 --- Ship it! Ship It! - David McLaughlin On Aug. 21, 2014, 8:44 p.m., Mark Chu-Carroll wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24953/ > --- > > (Updated Aug. 21, 2014, 8:44 p.m.) > > > Review request for Aurora, David McLaughlin and Maxim Khutornenko. > > > Bugs: aurora-659 > https://issues.apache.org/jira/browse/aurora-659 > > > Repository: aurora > > > Description > --- > > Also, add new unit test to prevent regressions. > > > Diffs > - > > src/main/python/apache/aurora/client/cli/context.py > 0e10589c2e8ab6decedb12122a0d8c7a81285e18 > src/main/python/apache/aurora/client/cli/options.py > bb61dbe14e46cfab609e26e36e4e7e176d9d0e50 > src/test/python/apache/aurora/client/cli/test_create.py > ca635bd49acec9596db67839451101a59ba4d761 > src/test/python/apache/aurora/client/cli/util.py > 552537add91a76b17736b456c78034a38940fa67 > > Diff: https://reviews.apache.org/r/24953/diff/ > > > Testing > --- > > [sun-wukong incubator-aurora (fix_bindings)]$ ./pants > src/test/python/apache/aurora/client/cli:all > Build operating on top level addresses: > set([BuildFileAddress(/Users/mchucarroll/Code/incubator-aurora/src/test/python/apache/aurora/client/cli/BUILD, > all)]) > = test session starts > == > platform darwin -- Python 2.7.5 -- py-1.4.23 -- pytest-2.6.1 > plugins: cov, timeout > collected 3 items > > src/test/python/apache/aurora/client/cli/test_config_noun.py ... > > === 3 passed in 0.97 seconds > === > = test session starts > == > platform darwin -- Python 2.7.5 -- py-1.4.23 -- pytest-2.6.1 > plugins: cov, timeout > collected 5 items > > src/test/python/apache/aurora/client/cli/test_sla.py . > > === 5 passed in 0.95 seconds > === > = test session starts > == > platform darwin -- Python 2.7.5 -- py-1.4.23 -- pytest-2.6.1 > plugins: cov, timeout > collected 4 items > > src/test/python/apache/aurora/client/cli/test_bridge.py > > === 4 passed in 0.03 seconds > === > = test session starts > == > platform darwin -- Python 2.7.5 -- py-1.4.23 -- pytest-2.6.1 > plugins: cov, timeout > collected 3 items > > src/test/python/apache/aurora/client/cli/test_task_run.py ... > > === 3 passed in 0.91 seconds > === > = test session starts > == > platform darwin -- Python 2.7.5 -- py-1.4.23 -- pytest-2.6.1 > plugins: cov, timeout > collected 7 items > > src/test/python/apache/aurora/client/cli/test_cron.py ... > > === 7 passed in 1.26 seconds > === > = test session starts > == > platform darwin -- Python 2.7.5 -- py-1.4.23 -- pytest-2.6.1 > plugins: cov, timeout > collected 45 items > > src/test/python/apache/aurora/client/cli/test_cancel_update.py .. > src/test/python/apache/aurora/client/cli/test_create.py ... > src/test/python/apache/aurora/client/cli/test_diff.py ... > src/test/python/apache/aurora/client/cli/test_kill.py . > src/test/python/apache/aurora/client/cli/test_open.py . > src/test/python/apache/aurora/client/cli/test_restart.py . > src/test/python/apache/aurora/client/cli/test_status.py ... > src/test/python/apache/aurora/client/cli/test_update.py ... > > == 45 passed in 7.91 seconds > === > = test session starts > == > platform darwin -- Python 2.7.5 -- py-1.4.23 -- pytest-2.6.1 > plugins: cov, timeout > collected 1 items > > src/test/python/apache/aurora/client/cli/test_inspect.py . > > === 1 passed in 0.73 seconds > === > = test session starts >
Re: Review Request 24982: Fixing checkstyle build break.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24982/#review51294 --- Ship it! Ship It! - David McLaughlin On Aug. 22, 2014, 3:49 p.m., Maxim Khutornenko wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24982/ > --- > > (Updated Aug. 22, 2014, 3:49 p.m.) > > > Review request for Aurora and David McLaughlin. > > > Bugs: AURORA-311 > https://issues.apache.org/jira/browse/AURORA-311 > > > Repository: aurora > > > Description > --- > > Fixing checkstyle build break introduced in > https://issues.apache.org/jira/browse/AURORA-311 > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/DriverFactory.java > f300ec900d5bc00782c90f9218ca29151a3c8bdc > > Diff: https://reviews.apache.org/r/24982/diff/ > > > Testing > --- > > gradle -Pq build > > > Thanks, > > Maxim Khutornenko > >
Re: Review Request 24995: Adding support for a pre-update quota check.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24995/#review51334 --- Ship it! lgtm. - David McLaughlin On Aug. 22, 2014, 8:55 p.m., Maxim Khutornenko wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24995/ > --- > > (Updated Aug. 22, 2014, 8:55 p.m.) > > > Review request for Aurora, David McLaughlin and Kevin Sweeney. > > > Bugs: AURORA-649 > https://issues.apache.org/jira/browse/AURORA-649 > > > Repository: aurora > > > Description > --- > > First step towards checking quota in startJobUpdate RPC. The final thrift > wiring is to follow. > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/quota/QuotaInfo.java > 7a1a09b94cfe6d4b58ef4e6ab958b71c4be1e9bd > src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java > 24145f52a17ec9b7458aa5578a5b99ef5b1cddab > src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java > 3dcb1c3e3d0138634b3d077c845ecc0d61d7fc0f > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java > f679f38f0fcff8abf2442cfcd885cd202ac55528 > src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java > a0ef57ec8af2fd6993eda338da14d1f4346ed3a4 > > src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java > fa611a913bad40a8c0515c578b394c460340e574 > > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java > 997ade0be58723d1d91725061bb128ccf45e25b4 > > Diff: https://reviews.apache.org/r/24995/diff/ > > > Testing > --- > > gradle -Pq build > > > Thanks, > > Maxim Khutornenko > >
Review Request 25158: Aurora Update UI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25158/ --- Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner. Bugs: AURORA-614 None Description --- Still working on trying to make the event timeline work, but going to show what I've got so far to get feedback. Will tidy up the code and post a review for it once I get a ship it for this. The main thing here is the instance status visualisation. I've mocked out data to show some use cases. Note: I have two job page versions here with a preview of an update in progress. Would be interested in getting feedback on which one people prefer. Diffs - Diff: https://reviews.apache.org/r/25158/diff/ Testing --- File Attachments Job Page with in progress update, using same template as update page https://reviews.apache.org/media/uploaded/files/2014/08/28/332fd237-eba5-4204-93e5-4e271f8b4f8c__job-page-in-progress-big-preview.png Job Update with smaller in progress update, and more conventional progress bar. https://reviews.apache.org/media/uploaded/files/2014/08/28/39f78052-348c-458c-956b-c3ba8ab1f642__job-progress-small-preview.png Update Page - medium sized job in progress https://reviews.apache.org/media/uploaded/files/2014/08/28/e6e7e5db-c5ff-4e68-a817-552243a706f5__update-page-in-progress-medium-job.png Update Page - Update completed view https://reviews.apache.org/media/uploaded/files/2014/08/28/fb8ed1c1-c90c-4c13-8464-6876ca811ad1__update-page-finished.png Update Page - partial update / canary view https://reviews.apache.org/media/uploaded/files/2014/08/28/ada5349c-84be-4838-9c20-cc99d5db3779__update-page-partial-canary.png Update Page - Update that has failed instances https://reviews.apache.org/media/uploaded/files/2014/08/28/7a8983f5-cc36-4219-ab9e-237680566c11__update-page-failures.png Update with skipped instances https://reviews.apache.org/media/uploaded/files/2014/08/28/d4283788-45b6-4564-9e9d-81327b8d3e06__update-page-skipped.png Update that had to be rolled back https://reviews.apache.org/media/uploaded/files/2014/08/28/0a968616-4199-4e34-bc37-3b66fff03eef__update-page-rolled-back.png Update where instances were removed https://reviews.apache.org/media/uploaded/files/2014/08/28/ca132beb-fcd4-4fe1-b682-6e229f632f45__update-page-instances-removed.png Update page - HUGE job, smaller viz https://reviews.apache.org/media/uploaded/files/2014/08/28/8e4d59f9-4644-46b7-a4b1-1a5de9538a80__update-page-huge-job.png Update Page - very small job https://reviews.apache.org/media/uploaded/files/2014/08/28/356db8be-e3d1-430a-ac35-3ff7f5855467__update-page-small-job.png Thanks, David McLaughlin
Re: Review Request 25158: Aurora Update UI
> On Aug. 28, 2014, 6:51 p.m., Joshua Cohen wrote: > > This looks nice! I'm assuming the grid is changes in real time as instances > > are updated? > > > > I think I prefer the big preview (the one with the grid as opposed to the > > progress bar). > > > > What are your thoughts on including the update summary from the finished > > view on the in progress view? I think that'd be useful information on an > > in-flight deploy, especially who kicked it off, when it started, and an > > elapsed timer...) > > Maxim Khutornenko wrote: > I actually like the small preview with progress bar more. Having instance > break-down could be overwhelming for larger jobs. Owner, progress and elapsed > time should be enough here saving the instance details "wow" factor for the > update details page. > > Also, is there a sample page for an update with added instances? Re: Joshua's points In-flight deploy does contain the username and when it was started? It's just not as fancy because that display style only makes sense as a range. Re: Maxim No page for added instances - added instances are not a terminal state. They are just treated as pending update targets. > On Aug. 28, 2014, 6:51 p.m., Joshua Cohen wrote: > > File Attachment: Update page - HUGE job, smaller viz - > > update-page-huge-job.png > > <https://reviews.apache.org/r/25158/#fcomment26> > > > > Beyond a certain point, instead of making the boxes smaller and smaller > > (or having this section take up more and more real estate), does it make > > sense to set a lower bound for size (and an upper bound for overall > > vertical hiehgt) and simply let each box represent multiple instances? Just to be clear - there are only three classes of size and it's a pretty simple heuristic. The 'big' boxes (the one with instance ids displayed) are for jobs with less than 20 instances. Between 20 and 1000 you get the medium sized visualisation, and then 1000+ you get the smallest boxes (as shown here). I did make probably a bad assumption that 3~5k was hitting the upper bound of job size. At the scale of thousands of instances, it's definitely going to be better to skip any kind of one-size-fits-all viz and maybe only show 'interesting' instances. > On Aug. 28, 2014, 6:51 p.m., Joshua Cohen wrote: > > File Attachment: Update with skipped instances - update-page-skipped.png > > <https://reviews.apache.org/r/25158/#fcomment27> > > > > This brighter green for skipped feels off to me. It looks *more* > > successful than the actual successful updates. Yeah, agreed. I'll tinker with that color when posting the code review. - David ------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25158/#review51808 --- On Aug. 28, 2014, 6:13 p.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25158/ > --- > > (Updated Aug. 28, 2014, 6:13 p.m.) > > > Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner. > > > Bugs: AURORA-614 > None > > > Description > --- > > Still working on trying to make the event timeline work, but going to show > what I've got so far to get feedback. Will tidy up the code and post a review > for it once I get a ship it for this. > > The main thing here is the instance status visualisation. I've mocked out > data to show some use cases. > > Note: I have two job page versions here with a preview of an update in > progress. Would be interested in getting feedback on which one people prefer. > > > Diffs > - > > > Diff: https://reviews.apache.org/r/25158/diff/ > > > Testing > --- > > > File Attachments > > > Job Page with in progress update, using same template as update page > > https://reviews.apache.org/media/uploaded/files/2014/08/28/332fd237-eba5-4204-93e5-4e271f8b4f8c__job-page-in-progress-big-preview.png > Job Update with smaller in progress update, and more conventional progress > bar. > > https://reviews.apache.org/media/uploaded/files/2014/08/28/39f78052-348c-458c-956b-c3ba8ab1f642__job-progress-small-preview.png > Update Page - medium sized job in progress > > https://reviews.apache.org/media/uploaded/files/2014/08/28/e6e7e5db-c5ff-4e68-a817-552243a706f5__update-page-in
Re: Review Request 25285: Upgrade to latest in jetty 7.x series.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25285/#review52203 --- Ship it! Ship It! - David McLaughlin On Sept. 3, 2014, 5:23 a.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25285/ > --- > > (Updated Sept. 3, 2014, 5:23 a.m.) > > > Review request for Aurora, David McLaughlin and Kevin Sweeney. > > > Bugs: AURORA-679 > https://issues.apache.org/jira/browse/AURORA-679 > > > Repository: aurora > > > Description > --- > > Long story short: this change adds code, but enables us to remove code in a > follow-up. > > This change brings us to Jetty 7, which is the newest jetty that still uses > servlet-api 2.5. Jetty 8/9 will require a larger yak shave since it pulls a > dependency thread all the way to guice (servlet-api -> jersey-guice -> > guice-servlet -> guice). > > This is also a small step towards insulating ourselves from transitive > dependencies pulled in from twitter commons. > > [1] http://wiki.eclipse.org/Jetty/Starting/Porting_to_Jetty_7 > > > Diffs > - > > build.gradle 66c8205c8354048c398623edf7ba75e721376b01 > src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java > de49a1c5497e32ee4db944412e5c87807c742d3c > src/main/java/org/apache/aurora/scheduler/http/RequestLogger.java > PRE-CREATION > src/test/java/org/apache/aurora/scheduler/http/RequestLoggerTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/25285/diff/ > > > Testing > --- > > ./gradlew build -Pq > > Ran the scheduler in vagrant, verified index page works, request logging > works, and all pages linked from the index page work. > > > Thanks, > > Bill Farner > >
Re: Review Request 25297: Make config-file an optional parameter for "job restart."
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25297/#review52208 --- Ship it! Ship It! - David McLaughlin On Sept. 3, 2014, 2:07 p.m., Mark Chu-Carroll wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25297/ > --- > > (Updated Sept. 3, 2014, 2:07 p.m.) > > > Review request for Aurora, David McLaughlin and Maxim Khutornenko. > > > Bugs: aurora-673 > https://issues.apache.org/jira/browse/aurora-673 > > > Repository: aurora > > > Description > --- > > Make config-file an optional parameter for "job restart." > > > Diffs > - > > src/main/python/apache/aurora/client/cli/jobs.py > ebc22aaa5a8aed311897b3ce9632b6f7175b6080 > src/test/python/apache/aurora/client/cli/test_restart.py > a1e7a5a94a2d336239df98e2600658b97c546901 > src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh > 14a0b62c1b06179211d79f35a9c6df162c9a5999 > > Diff: https://reviews.apache.org/r/25297/diff/ > > > Testing > --- > > - Ran all unit tests successfully. > - Ran end-to-end tests successfully. > - > > > Thanks, > > Mark Chu-Carroll > >
Re: Review Request 25309: Fix output formatting error in "job status".
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25309/#review52257 --- Ship it! Ship It! - David McLaughlin On Sept. 3, 2014, 6:17 p.m., Mark Chu-Carroll wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25309/ > --- > > (Updated Sept. 3, 2014, 6:17 p.m.) > > > Review request for Aurora, David McLaughlin and Maxim Khutornenko. > > > Bugs: aurora-672 > https://issues.apache.org/jira/browse/aurora-672 > > > Repository: aurora > > > Description > --- > > Fix output formatting error in "job status". > > > Diffs > - > > src/main/python/apache/aurora/client/cli/jobs.py > ebc22aaa5a8aed311897b3ce9632b6f7175b6080 > src/test/python/apache/aurora/client/cli/test_status.py > 311fac02af32e0ed687489a2352164effb4dba96 > > Diff: https://reviews.apache.org/r/25309/diff/ > > > Testing > --- > > Ran unit tests; added new test cases. > > > Thanks, > > Mark Chu-Carroll > >
Re: Review Request 25357: Adding support for per-job task status metrics.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25357/#review52379 --- Ship it! Ship It! - David McLaughlin On Sept. 4, 2014, 9:42 p.m., Maxim Khutornenko wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25357/ > --- > > (Updated Sept. 4, 2014, 9:42 p.m.) > > > Review request for Aurora, David McLaughlin and Bill Farner. > > > Bugs: AURORA-685 > https://issues.apache.org/jira/browse/AURORA-685 > > > Repository: aurora > > > Description > --- > > Currently tracking LOST and FAILED states. > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/TaskVars.java > 6654c1675ac9f5f7d481e115cea7c224fb212467 > src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java > d02714c846a521ff9ac3e53d991731314e714ae2 > > Diff: https://reviews.apache.org/r/25357/diff/ > > > Testing > --- > > gradle -Pq build > > > Thanks, > > Maxim Khutornenko > >
Re: Review Request 25337: Preserve executor HealthCheckerThread name
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25337/#review52397 --- Ship it! lgtm. - David McLaughlin On Sept. 5, 2014, 1:20 a.m., Joe Smith wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25337/ > --- > > (Updated Sept. 5, 2014, 1:20 a.m.) > > > Review request for Aurora, David McLaughlin, Maxim Khutornenko, and Brian > Wickman. > > > Bugs: AURORA-682 > https://issues.apache.org/jira/browse/AURORA-682 > > > Repository: aurora > > > Description > --- > > Preserve executor HealthCheckerThread name > > > Diffs > - > > src/main/python/apache/aurora/executor/common/health_checker.py > 603fff35b839c6f53d9379ec047d7d8135a1c65b > src/test/python/apache/aurora/executor/common/BUILD > 3229facf40070929adabb57fef667ab11bf3d1ec > src/test/python/apache/aurora/executor/common/fixtures.py PRE-CREATION > src/test/python/apache/aurora/executor/common/test_health_checker.py > 490d4c8b5c434f9d6f032d931e35c483b3a5b676 > src/test/python/apache/aurora/executor/common/test_task_info.py > 344fd675c9a1ef9c88e39c16ac0f1dd50a9c1632 > > Diff: https://reviews.apache.org/r/25337/diff/ > > > Testing > --- > > ###STILL RUNNING E2E TESTS > > [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ > ./build-support/python/isort-run > [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ > ./build-support/python/checkstyle-check > [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./pants > ./src/test/python/apache/aurora/executor:all > src.test.python.apache.aurora.executor.common.announcer > . SUCCESS > src.test.python.apache.aurora.executor.common.directory_sandbox > . SUCCESS > src.test.python.apache.aurora.executor.common.executor_timeout > . SUCCESS > src.test.python.apache.aurora.executor.common.health_checker > . SUCCESS > src.test.python.apache.aurora.executor.common.status_checker > . SUCCESS > src.test.python.apache.aurora.executor.common.task_info > . SUCCESS > src.test.python.apache.aurora.executor.executor_base > . SUCCESS > src.test.python.apache.aurora.executor.executor_detector > . SUCCESS > src.test.python.apache.aurora.executor.executor_vars > . SUCCESS > src.test.python.apache.aurora.executor.gc_executor > . SUCCESS > src.test.python.apache.aurora.executor.status_manager > . SUCCESS > src.test.python.apache.aurora.executor.thermos_executor > . SUCCESS > src.test.python.apache.aurora.executor.thermos_task_runner > . SUCCESS > > > Thanks, > > Joe Smith > >
Re: Review Request 25391: Add information about log initialisation step.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25391/#review52474 --- Ship it! Ship It! - David McLaughlin On Sept. 5, 2014, 3:45 p.m., Joseph Glanville wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25391/ > --- > > (Updated Sept. 5, 2014, 3:45 p.m.) > > > Review request for Aurora, David McLaughlin and Bill Farner. > > > Repository: aurora > > > Description > --- > > This tripped me up when doing a manual deployment without the help of the > Vagrant scripts. > I am fairly certain it also pops up on IRC fairly often, it would be good to > document this here so newcomers don't get stuck on it. > > > Diffs > - > > docs/deploying-aurora-scheduler.md 25767fe > > Diff: https://reviews.apache.org/r/25391/diff/ > > > Testing > --- > > > Thanks, > > Joseph Glanville > >
Review Request 25259: Add update information to the scheduler UI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/ --- Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner. Bugs: AURORA-614 https://issues.apache.org/jira/browse/AURORA-614 Repository: aurora Description --- Adds update history to the job page. Adds an update details page. Diffs - src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java de49a1c5497e32ee4db944412e5c87807c742d3c src/main/resources/org/apache/aurora/scheduler/http/ui/breadcrumb.html c780b0fe98863b5421824a9652a7202da9f4302a src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css 2a752313cb8ae404605a9458b33237a911eec061 src/main/resources/org/apache/aurora/scheduler/http/ui/job.html e21dee015897eee62ade8f74e26f042b8e2be734 src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js fb3b5b12121a6e8a30dbf6fe069557f69a563408 src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 3477b7e667459665af9d9dc9d2456793822bc7f7 src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js 7f05a552f3786adb115ff9648f4fadce968230e9 src/main/resources/org/apache/aurora/scheduler/http/ui/js/filters.js df2806481fc1c2f263d3afd9b21247e97a18ed57 src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js bfd360de45c75441743c8ba24a5c445f4146dba6 src/main/resources/org/apache/aurora/scheduler/http/ui/timeDisplay.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/update.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html PRE-CREATION Diff: https://reviews.apache.org/r/25259/diff/ Testing --- ./gradlew jsHint File Attachments job page https://reviews.apache.org/media/uploaded/files/2014/09/09/531eca81-a0ba-4438-8bd6-4b50d97b0270__job-progress-small-preview.png update page https://reviews.apache.org/media/uploaded/files/2014/09/09/8e3f2950-7d7e-404e-bca9-6c472b5218f7__update-page-finished.png Thanks, David McLaughlin
Re: Review Request 25450: Fix broken "large update" warning.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25450/#review52777 --- Ship it! Ship It! - David McLaughlin On Sept. 9, 2014, 7:43 p.m., Mark Chu-Carroll wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25450/ > --- > > (Updated Sept. 9, 2014, 7:43 p.m.) > > > Review request for Aurora, David McLaughlin and Maxim Khutornenko. > > > Bugs: aurora-5860 > https://issues.apache.org/jira/browse/aurora-5860 > > > Repository: aurora > > > Description > --- > > At some point, the "large update" warning logic was revised so that the > warning shows whenever you do an update that either: > - the updated config had more than 4x as many instances as the running one; or > - the updated config had less than 4x as many instances as the running one. > > (Seriously: either local >= 4 * remote or local <= 4 * remote". And running > git blame says it's all my fault.) > > This fixes it: the correct logic is: > - the updated config has more than 4x as many as running; or > - the updated config has less than 1/4th as many as running. > > > Diffs > - > > src/main/python/apache/aurora/client/cli/__init__.py > 6e553d8af459e575b2d62282a3bc0d1e266203d8 > src/main/python/apache/aurora/client/cli/jobs.py > 8f349c09637c16e2499e85f2dc96eb7ccffd0aaf > src/test/python/apache/aurora/client/cli/test_update.py > 8b7d11202b35deb09a248cfe0a96458fb70c > src/test/python/apache/aurora/client/cli/util.py > 95a2123e127c9811fd2305e71cfc5c7c4376f904 > > Diff: https://reviews.apache.org/r/25450/diff/ > > > Testing > --- > > Added new unit tests. > > > Thanks, > > Mark Chu-Carroll > >
Re: Review Request 25259: Add update information to the scheduler UI
> On Sept. 9, 2014, 1:45 a.m., Maxim Khutornenko wrote: > > src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js, > > line 277 > > <https://reviews.apache.org/r/25259/diff/1/?file=683446#file683446line277> > > > > This seems to be too short to make any visual difference but could be a > > perf hit. How about 10-15sec instead? > > > > Also, is there any visual cue that the progress is auto-updated? > > Maxim Khutornenko wrote: > Also, will the auto-update get suspended when the page is out of focus > (e.g. user switches to another tab or hides the browser)? Would be great if > it does. No visual cue that it's auto-updated. It could be something to add in the future, or do you see it as a blocker for this? I picked 3s off the top of my head. 10s sounds like a good alternative. Auto-update being suspended when the page is out of focus doesn't seem like a desired feature. When I come back to the tab the page is always in the same state as I last saw it. There is definitely a use case of tabbing in and out to see how an update is going. > On Sept. 9, 2014, 1:45 a.m., Maxim Khutornenko wrote: > > src/main/resources/org/apache/aurora/scheduler/http/ui/js/filters.js, line > > 54 > > <https://reviews.apache.org/r/25259/diff/1/?file=683448#file683448line54> > > > > Should not this be ROLL BACK PAUSED instead? I figured the user didn't really care about what type of pause it was, and the previous state is right there in the status history. Thoughts? - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/#review52661 --- On Sept. 9, 2014, 1:05 a.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25259/ > --- > > (Updated Sept. 9, 2014, 1:05 a.m.) > > > Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner. > > > Bugs: AURORA-614 > https://issues.apache.org/jira/browse/AURORA-614 > > > Repository: aurora > > > Description > --- > > Adds update history to the job page. Adds an update details page. > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java > de49a1c5497e32ee4db944412e5c87807c742d3c > src/main/resources/org/apache/aurora/scheduler/http/ui/breadcrumb.html > c780b0fe98863b5421824a9652a7202da9f4302a > src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css > 2a752313cb8ae404605a9458b33237a911eec061 > src/main/resources/org/apache/aurora/scheduler/http/ui/job.html > e21dee015897eee62ade8f74e26f042b8e2be734 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js > fb3b5b12121a6e8a30dbf6fe069557f69a563408 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js > 3477b7e667459665af9d9dc9d2456793822bc7f7 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js > 7f05a552f3786adb115ff9648f4fadce968230e9 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/filters.js > df2806481fc1c2f263d3afd9b21247e97a18ed57 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js > bfd360de45c75441743c8ba24a5c445f4146dba6 > src/main/resources/org/apache/aurora/scheduler/http/ui/timeDisplay.html > PRE-CREATION > src/main/resources/org/apache/aurora/scheduler/http/ui/update.html > PRE-CREATION > src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html > PRE-CREATION > > Diff: https://reviews.apache.org/r/25259/diff/ > > > Testing > --- > > ./gradlew jsHint > > > File Attachments > > > job page > > https://reviews.apache.org/media/uploaded/files/2014/09/09/531eca81-a0ba-4438-8bd6-4b50d97b0270__job-progress-small-preview.png > update page > > https://reviews.apache.org/media/uploaded/files/2014/09/09/8e3f2950-7d7e-404e-bca9-6c472b5218f7__update-page-finished.png > > > Thanks, > > David McLaughlin > >
Re: Review Request 25259: Add update information to the scheduler UI
> On Sept. 9, 2014, 6:43 p.m., Kevin Sweeney wrote: > > src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js, > > line 277 > > <https://reviews.apache.org/r/25259/diff/1/?file=683446#file683446line277> > > > > Extract this as a configuration variable? Fixed. > On Sept. 9, 2014, 6:43 p.m., Kevin Sweeney wrote: > > src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js, > > line 411 > > <https://reviews.apache.org/r/25259/diff/1/?file=683446#file683446line411> > > > > DRY Fixed. > On Sept. 9, 2014, 6:43 p.m., Kevin Sweeney wrote: > > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js, line > > 156 > > <https://reviews.apache.org/r/25259/diff/1/?file=683449#file683449line156> > > > > Since this has no dependencies you can make it .constant This is defined how taskUtil is for consistency. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/#review52665 --- On Sept. 9, 2014, 1:05 a.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25259/ > --- > > (Updated Sept. 9, 2014, 1:05 a.m.) > > > Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner. > > > Bugs: AURORA-614 > https://issues.apache.org/jira/browse/AURORA-614 > > > Repository: aurora > > > Description > --- > > Adds update history to the job page. Adds an update details page. > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java > de49a1c5497e32ee4db944412e5c87807c742d3c > src/main/resources/org/apache/aurora/scheduler/http/ui/breadcrumb.html > c780b0fe98863b5421824a9652a7202da9f4302a > src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css > 2a752313cb8ae404605a9458b33237a911eec061 > src/main/resources/org/apache/aurora/scheduler/http/ui/job.html > e21dee015897eee62ade8f74e26f042b8e2be734 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js > fb3b5b12121a6e8a30dbf6fe069557f69a563408 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js > 3477b7e667459665af9d9dc9d2456793822bc7f7 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js > 7f05a552f3786adb115ff9648f4fadce968230e9 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/filters.js > df2806481fc1c2f263d3afd9b21247e97a18ed57 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js > bfd360de45c75441743c8ba24a5c445f4146dba6 > src/main/resources/org/apache/aurora/scheduler/http/ui/timeDisplay.html > PRE-CREATION > src/main/resources/org/apache/aurora/scheduler/http/ui/update.html > PRE-CREATION > src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html > PRE-CREATION > > Diff: https://reviews.apache.org/r/25259/diff/ > > > Testing > --- > > ./gradlew jsHint > > > File Attachments > > > job page > > https://reviews.apache.org/media/uploaded/files/2014/09/09/531eca81-a0ba-4438-8bd6-4b50d97b0270__job-progress-small-preview.png > update page > > https://reviews.apache.org/media/uploaded/files/2014/09/09/8e3f2950-7d7e-404e-bca9-6c472b5218f7__update-page-finished.png > > > Thanks, > > David McLaughlin > >
Re: Review Request 25259: Add update information to the scheduler UI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/ --- (Updated Sept. 9, 2014, 11:32 p.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner. Changes --- rb feedback. Bugs: AURORA-614 https://issues.apache.org/jira/browse/AURORA-614 Repository: aurora Description --- Adds update history to the job page. Adds an update details page. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java de49a1c5497e32ee4db944412e5c87807c742d3c src/main/resources/org/apache/aurora/scheduler/http/ui/breadcrumb.html c780b0fe98863b5421824a9652a7202da9f4302a src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css 2a752313cb8ae404605a9458b33237a911eec061 src/main/resources/org/apache/aurora/scheduler/http/ui/job.html e21dee015897eee62ade8f74e26f042b8e2be734 src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js fb3b5b12121a6e8a30dbf6fe069557f69a563408 src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 3477b7e667459665af9d9dc9d2456793822bc7f7 src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js 7f05a552f3786adb115ff9648f4fadce968230e9 src/main/resources/org/apache/aurora/scheduler/http/ui/js/filters.js df2806481fc1c2f263d3afd9b21247e97a18ed57 src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js bfd360de45c75441743c8ba24a5c445f4146dba6 src/main/resources/org/apache/aurora/scheduler/http/ui/timeDisplay.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/update.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html PRE-CREATION src/main/thrift/org/apache/aurora/gen/api.thrift 2b376d663b3bc9264965db58ef89de59b10169ad Diff: https://reviews.apache.org/r/25259/diff/ Testing --- ./gradlew jsHint File Attachments job page https://reviews.apache.org/media/uploaded/files/2014/09/09/531eca81-a0ba-4438-8bd6-4b50d97b0270__job-progress-small-preview.png update page https://reviews.apache.org/media/uploaded/files/2014/09/09/8e3f2950-7d7e-404e-bca9-6c472b5218f7__update-page-finished.png Thanks, David McLaughlin
Re: Review Request 25259: Add update information to the scheduler UI
> On Sept. 9, 2014, 1:45 a.m., Maxim Khutornenko wrote: > > src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js, > > line 277 > > <https://reviews.apache.org/r/25259/diff/1/?file=683446#file683446line277> > > > > This seems to be too short to make any visual difference but could be a > > perf hit. How about 10-15sec instead? > > > > Also, is there any visual cue that the progress is auto-updated? > > Maxim Khutornenko wrote: > Also, will the auto-update get suspended when the page is out of focus > (e.g. user switches to another tab or hides the browser)? Would be great if > it does. > > David McLaughlin wrote: > No visual cue that it's auto-updated. It could be something to add in the > future, or do you see it as a blocker for this? I picked 3s off the top of my > head. 10s sounds like a good alternative. > > > Auto-update being suspended when the page is out of focus doesn't seem > like a desired feature. When I come back to the tab the page is always in the > same state as I last saw it. There is definitely a use case of tabbing in and > out to see how an update is going. > > Maxim Khutornenko wrote: > | When I come back to the tab the page is always in the same state as I > last saw it. > Is it possible to suspend auto-update on lost focus and resume on > re-acquiring it? Not sure if there is a high level event available for it but > would be great to avoid wasted expensive queries on the scheduler if no one > is watching the page. The problem with tabs/windows is they can be inactive but still visible in your screen. e.g. there's no way to know if the inactive window is hidden behind another window or still visible on a second screen. > On Sept. 9, 2014, 1:45 a.m., Maxim Khutornenko wrote: > > src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js, > > line 411 > > <https://reviews.apache.org/r/25259/diff/1/?file=683446#file683446line411> > > > > Same here. Fixed. > On Sept. 9, 2014, 1:45 a.m., Maxim Khutornenko wrote: > > src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html, > > line 31 > > <https://reviews.apache.org/r/25259/diff/1/?file=683452#file683452line31> > > > > Missing period. Fixed. > On Sept. 9, 2014, 1:45 a.m., Maxim Khutornenko wrote: > > src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html, > > line 32 > > <https://reviews.apache.org/r/25259/diff/1/?file=683452#file683452line32> > > > > s/Instances/Instance Fixed. > On Sept. 9, 2014, 1:45 a.m., Maxim Khutornenko wrote: > > src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html, > > line 41 > > <https://reviews.apache.org/r/25259/diff/1/?file=683452#file683452line41> > > > > Missing period. Fixed. > On Sept. 9, 2014, 1:45 a.m., Maxim Khutornenko wrote: > > src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html, > > line 61 > > <https://reviews.apache.org/r/25259/diff/1/?file=683452#file683452line61> > > > > s/to watch to watch/to watch > > > > Just realized it's copied from api.thrift. Mind fixing it there as well? Fixed. > On Sept. 9, 2014, 1:45 a.m., Maxim Khutornenko wrote: > > src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html, > > line 62 > > <https://reviews.apache.org/r/25259/diff/1/?file=683452#file683452line62> > > > > Suggest renaming to "Min Waiting Time In Running" to better reflect on > > the setting. Fixed. > On Sept. 9, 2014, 1:45 a.m., Maxim Khutornenko wrote: > > src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html, > > line 81 > > <https://reviews.apache.org/r/25259/diff/1/?file=683452#file683452line81> > > > > s/set/subset? > > > > Also, suggest adding: "All instances will be affected if this is not > > set." Fixed. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/#review52661 --- On Sept. 9, 2014, 11:32 p.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25259/ > --- > > (Updated Sept. 9, 2014, 11:32 p.m.) > > > Review request for Aurora, Joshua Co
Re: Review Request 25259: Add update information to the scheduler UI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/ --- (Updated Sept. 9, 2014, 11:50 p.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner. Changes --- Fixed typo. Bugs: AURORA-614 https://issues.apache.org/jira/browse/AURORA-614 Repository: aurora Description --- Adds update history to the job page. Adds an update details page. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java de49a1c5497e32ee4db944412e5c87807c742d3c src/main/resources/org/apache/aurora/scheduler/http/ui/breadcrumb.html c780b0fe98863b5421824a9652a7202da9f4302a src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css 2a752313cb8ae404605a9458b33237a911eec061 src/main/resources/org/apache/aurora/scheduler/http/ui/job.html e21dee015897eee62ade8f74e26f042b8e2be734 src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js fb3b5b12121a6e8a30dbf6fe069557f69a563408 src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 3477b7e667459665af9d9dc9d2456793822bc7f7 src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js 7f05a552f3786adb115ff9648f4fadce968230e9 src/main/resources/org/apache/aurora/scheduler/http/ui/js/filters.js df2806481fc1c2f263d3afd9b21247e97a18ed57 src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js bfd360de45c75441743c8ba24a5c445f4146dba6 src/main/resources/org/apache/aurora/scheduler/http/ui/timeDisplay.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/update.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html PRE-CREATION src/main/thrift/org/apache/aurora/gen/api.thrift 2b376d663b3bc9264965db58ef89de59b10169ad Diff: https://reviews.apache.org/r/25259/diff/ Testing --- ./gradlew jsHint File Attachments job page https://reviews.apache.org/media/uploaded/files/2014/09/09/531eca81-a0ba-4438-8bd6-4b50d97b0270__job-progress-small-preview.png update page https://reviews.apache.org/media/uploaded/files/2014/09/09/8e3f2950-7d7e-404e-bca9-6c472b5218f7__update-page-finished.png Thanks, David McLaughlin
Re: Review Request 25259: Add update information to the scheduler UI
> On Sept. 9, 2014, 11:42 p.m., Maxim Khutornenko wrote: > > src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html, > > line 81 > > <https://reviews.apache.org/r/25259/diff/1-2/?file=683452#file683452line81> > > > > s/is this/if this D'oh. Fixed. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/#review52801 --- On Sept. 9, 2014, 11:50 p.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25259/ > --- > > (Updated Sept. 9, 2014, 11:50 p.m.) > > > Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner. > > > Bugs: AURORA-614 > https://issues.apache.org/jira/browse/AURORA-614 > > > Repository: aurora > > > Description > --- > > Adds update history to the job page. Adds an update details page. > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java > de49a1c5497e32ee4db944412e5c87807c742d3c > src/main/resources/org/apache/aurora/scheduler/http/ui/breadcrumb.html > c780b0fe98863b5421824a9652a7202da9f4302a > src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css > 2a752313cb8ae404605a9458b33237a911eec061 > src/main/resources/org/apache/aurora/scheduler/http/ui/job.html > e21dee015897eee62ade8f74e26f042b8e2be734 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js > fb3b5b12121a6e8a30dbf6fe069557f69a563408 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js > 3477b7e667459665af9d9dc9d2456793822bc7f7 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js > 7f05a552f3786adb115ff9648f4fadce968230e9 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/filters.js > df2806481fc1c2f263d3afd9b21247e97a18ed57 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js > bfd360de45c75441743c8ba24a5c445f4146dba6 > src/main/resources/org/apache/aurora/scheduler/http/ui/timeDisplay.html > PRE-CREATION > src/main/resources/org/apache/aurora/scheduler/http/ui/update.html > PRE-CREATION > src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html > PRE-CREATION > src/main/thrift/org/apache/aurora/gen/api.thrift > 2b376d663b3bc9264965db58ef89de59b10169ad > > Diff: https://reviews.apache.org/r/25259/diff/ > > > Testing > --- > > ./gradlew jsHint > > > File Attachments > > > job page > > https://reviews.apache.org/media/uploaded/files/2014/09/09/531eca81-a0ba-4438-8bd6-4b50d97b0270__job-progress-small-preview.png > update page > > https://reviews.apache.org/media/uploaded/files/2014/09/09/8e3f2950-7d7e-404e-bca9-6c472b5218f7__update-page-finished.png > > > Thanks, > > David McLaughlin > >
Re: Review Request 25505: Make "aurora job status" JSON output more friendly.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25505/#review52931 --- Ship it! Ship It! - David McLaughlin On Sept. 10, 2014, 7:34 p.m., Mark Chu-Carroll wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25505/ > --- > > (Updated Sept. 10, 2014, 7:34 p.m.) > > > Review request for Aurora, David McLaughlin and Maxim Khutornenko. > > > Bugs: aurora-666 > https://issues.apache.org/jira/browse/aurora-666 > > > Repository: aurora > > > Description > --- > > Use string names instead of numeric values for ScheduleStatus fields of > records rendered in JSON. > > > Diffs > - > > src/main/python/apache/aurora/client/cli/jobs.py > 38b5f6e306c2c8dd9aa8d98e21c6a628028ad899 > src/test/python/apache/aurora/client/cli/test_status.py > 721d9764190bdc4b1c5b65e416a039803b7c507c > > Diff: https://reviews.apache.org/r/25505/diff/ > > > Testing > --- > > Added new unit tests; all tests pass. > > > Thanks, > > Mark Chu-Carroll > >
Review Request 25552: Show reason for PENDING state in Scheduler UI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25552/ --- Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-495 https://issues.apache.org/jira/browse/AURORA-495 Repository: aurora Description --- Consumed the getPendingReason API call to add a message to any PENDING tasks. Diffs - src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js bfd360de45c75441743c8ba24a5c445f4146dba6 Diff: https://reviews.apache.org/r/25552/diff/ Testing --- ./gradlew jsHint Manual testing on local vagrant. Screenshots attached. File Attachments Screen Shot 2014-09-11 at 1.32.08 PM.png https://reviews.apache.org/media/uploaded/files/2014/09/11/e44e3909-ad9b-4c4c-a3e6-6b6b3aedaa3c__Screen_Shot_2014-09-11_at_1.32.08_PM.png Thanks, David McLaughlin
Re: Review Request 25548: Apply GzipFilter to POSTs as well as GETs
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25548/#review53105
---
Ship it!
Ship It!
- David McLaughlin
On Sept. 11, 2014, 6:20 p.m., Joshua Cohen wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25548/
> ---
>
> (Updated Sept. 11, 2014, 6:20 p.m.)
>
>
> Review request for Aurora, David McLaughlin and Bill Farner.
>
>
> Bugs: AURORA-697
> https://issues.apache.org/jira/browse/AURORA-697
>
>
> Repository: aurora
>
>
> Description
> ---
>
> Apply GzipFilter to POSTs as well as GETs
>
>
> Diffs
> -
>
> src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java
> 83ba0e49436034c8b6f9f736c60a726686096362
> src/test/java/org/apache/aurora/scheduler/http/ServletFilterTest.java
> 2c31df6d0ac1c58e02b13d61ba9f511b781a20a7
>
> Diff: https://reviews.apache.org/r/25548/diff/
>
>
> Testing
> ---
>
> ./gradlew build -Pq
>
> Also verified content-encoding against response from scheduler in vagrant
> image:
>
> $ curl -s -D - -o /dev/null -H "Accept-Encoding: gzip" -X POST -d @- \
> > http://192.168.33.7:8081/api < > "Content-Encoding" |awk '{print $2}' \
> > |tr '[:upper:]' '[:lower]'
> > [1,"getJobSummary",1,0,{"1":{"str":"vagrant"}}]
> > EOF
> gzip
>
>
> Thanks,
>
> Joshua Cohen
>
>
Re: Review Request 25548: Apply GzipFilter to POSTs as well as GETs
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25548/#review53109
---
Pushed to Apache master.
- David McLaughlin
On Sept. 11, 2014, 6:20 p.m., Joshua Cohen wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25548/
> ---
>
> (Updated Sept. 11, 2014, 6:20 p.m.)
>
>
> Review request for Aurora, David McLaughlin and Bill Farner.
>
>
> Bugs: AURORA-697
> https://issues.apache.org/jira/browse/AURORA-697
>
>
> Repository: aurora
>
>
> Description
> ---
>
> Apply GzipFilter to POSTs as well as GETs
>
>
> Diffs
> -
>
> src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java
> 83ba0e49436034c8b6f9f736c60a726686096362
> src/test/java/org/apache/aurora/scheduler/http/ServletFilterTest.java
> 2c31df6d0ac1c58e02b13d61ba9f511b781a20a7
>
> Diff: https://reviews.apache.org/r/25548/diff/
>
>
> Testing
> ---
>
> ./gradlew build -Pq
>
> Also verified content-encoding against response from scheduler in vagrant
> image:
>
> $ curl -s -D - -o /dev/null -H "Accept-Encoding: gzip" -X POST -d @- \
> > http://192.168.33.7:8081/api < > "Content-Encoding" |awk '{print $2}' \
> > |tr '[:upper:]' '[:lower]'
> > [1,"getJobSummary",1,0,{"1":{"str":"vagrant"}}]
> > EOF
> gzip
>
>
> Thanks,
>
> Joshua Cohen
>
>
Re: Review Request 25391: Add information about log initialisation step.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25391/#review53110 --- Thanks Joseph! Pushed to master. - David McLaughlin On Sept. 5, 2014, 3:45 p.m., Joseph Glanville wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25391/ > --- > > (Updated Sept. 5, 2014, 3:45 p.m.) > > > Review request for Aurora, David McLaughlin and Bill Farner. > > > Repository: aurora > > > Description > --- > > This tripped me up when doing a manual deployment without the help of the > Vagrant scripts. > I am fairly certain it also pops up on IRC fairly often, it would be good to > document this here so newcomers don't get stuck on it. > > > Diffs > - > > docs/deploying-aurora-scheduler.md 25767fe > > Diff: https://reviews.apache.org/r/25391/diff/ > > > Testing > --- > > > Thanks, > > Joseph Glanville > >
Re: Review Request 25552: Show reason for PENDING state in Scheduler UI
> On Sept. 11, 2014, 8:45 p.m., Joshua Cohen wrote: > > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js, line > > 90 > > <https://reviews.apache.org/r/25552/diff/1/?file=686507#file686507line90> > > > > I think this could just be: > > > > _.indexBy(reasons, 'taskId') Good catch. Fixed. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25552/#review53106 ------- On Sept. 11, 2014, 9:55 p.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25552/ > --- > > (Updated Sept. 11, 2014, 9:55 p.m.) > > > Review request for Aurora, Joshua Cohen and Maxim Khutornenko. > > > Bugs: AURORA-495 > https://issues.apache.org/jira/browse/AURORA-495 > > > Repository: aurora > > > Description > --- > > Consumed the getPendingReason API call to add a message to any PENDING tasks. > > > Diffs > - > > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js > bfd360de45c75441743c8ba24a5c445f4146dba6 > > Diff: https://reviews.apache.org/r/25552/diff/ > > > Testing > --- > > ./gradlew jsHint > > > Manual testing on local vagrant. Screenshots attached. > > > File Attachments > > > Screen Shot 2014-09-11 at 1.32.08 PM.png > > https://reviews.apache.org/media/uploaded/files/2014/09/11/e44e3909-ad9b-4c4c-a3e6-6b6b3aedaa3c__Screen_Shot_2014-09-11_at_1.32.08_PM.png > > > Thanks, > > David McLaughlin > >
Re: Review Request 25552: Show reason for PENDING state in Scheduler UI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25552/ --- (Updated Sept. 11, 2014, 9:55 p.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Changes --- Simplified underscore accessor. Bugs: AURORA-495 https://issues.apache.org/jira/browse/AURORA-495 Repository: aurora Description --- Consumed the getPendingReason API call to add a message to any PENDING tasks. Diffs (updated) - src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js bfd360de45c75441743c8ba24a5c445f4146dba6 Diff: https://reviews.apache.org/r/25552/diff/ Testing --- ./gradlew jsHint Manual testing on local vagrant. Screenshots attached. File Attachments Screen Shot 2014-09-11 at 1.32.08 PM.png https://reviews.apache.org/media/uploaded/files/2014/09/11/e44e3909-ad9b-4c4c-a3e6-6b6b3aedaa3c__Screen_Shot_2014-09-11_at_1.32.08_PM.png Thanks, David McLaughlin
Re: Review Request 25582: Fix error in client "task ssh" command when the job isn't found.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25582/#review53182 --- Ship it! src/main/python/apache/aurora/client/cli/task.py <https://reviews.apache.org/r/25582/#comment92643> Is this a debug line? - David McLaughlin On Sept. 12, 2014, 2:34 p.m., Mark Chu-Carroll wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25582/ > --- > > (Updated Sept. 12, 2014, 2:34 p.m.) > > > Review request for Aurora, David McLaughlin and Zameer Manji. > > > Bugs: aurora-706 > https://issues.apache.org/jira/browse/aurora-706 > > > Repository: aurora > > > Description > --- > > Fix error in client "task ssh" command when the job isn't found. > > > Diffs > - > > src/main/python/apache/aurora/client/cli/task.py > 91175facdc8c9fd59ab66781f86ee8b5940a > src/main/python/apache/aurora/client/commands/ssh.py > 37a90089b72b86c82466f1819e7881a36bb2f214 > src/test/python/apache/aurora/client/cli/test_task_run.py > 8d9ef0543c1ab514d6f039ba63a1d417a4a90a1b > src/test/python/apache/aurora/client/commands/test_ssh.py > 4070b710b005c91fe08dd7906cd93bf3a8cdba9e > > Diff: https://reviews.apache.org/r/25582/diff/ > > > Testing > --- > > Added new tests to catch this case; > Ran all client unit tests, all tests pass. > > > Thanks, > > Mark Chu-Carroll > >
Re: Review Request 25582: Fix error in client "task ssh" command when the job isn't found.
> On Sept. 12, 2014, 5:09 p.m., Joe Smith wrote: > > src/test/python/apache/aurora/client/cli/test_task_run.py, line 228 > > <https://reviews.apache.org/r/25582/diff/1/?file=687672#file687672line228> > > > > "Test the ssh command for proper behavior when no tasks are found > > within a job" or something, I think > > Joshua Cohen wrote: > I'd go so far as to suggest that docstrings on test methods are probably > not necessary. This exemplifies why, they just get copied/pasted from other > tests and end up not accurately describing what each test does. I'd vote for > descriptive test case names and do away with docstrings entirely. +1 - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25582/#review53184 --- On Sept. 12, 2014, 5:17 p.m., Mark Chu-Carroll wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25582/ > --- > > (Updated Sept. 12, 2014, 5:17 p.m.) > > > Review request for Aurora, David McLaughlin and Zameer Manji. > > > Bugs: aurora-706 > https://issues.apache.org/jira/browse/aurora-706 > > > Repository: aurora > > > Description > --- > > Fix error in client "task ssh" command when the job isn't found. > > > Diffs > - > > src/main/python/apache/aurora/client/cli/task.py > 91175facdc8c9fd59ab66781f86ee8b5940a > src/main/python/apache/aurora/client/commands/ssh.py > 37a90089b72b86c82466f1819e7881a36bb2f214 > src/test/python/apache/aurora/client/cli/test_task_run.py > 8d9ef0543c1ab514d6f039ba63a1d417a4a90a1b > src/test/python/apache/aurora/client/commands/test_ssh.py > 4070b710b005c91fe08dd7906cd93bf3a8cdba9e > > Diff: https://reviews.apache.org/r/25582/diff/ > > > Testing > --- > > Added new tests to catch this case; > Ran all client unit tests, all tests pass. > > > Thanks, > > Mark Chu-Carroll > >
Re: Review Request 25259: Add update information to the scheduler UI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/ --- (Updated Sept. 12, 2014, 10:32 p.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner. Changes --- rb feedback. Bugs: AURORA-614 https://issues.apache.org/jira/browse/AURORA-614 Repository: aurora Description --- Adds update history to the job page. Adds an update details page. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java de49a1c5497e32ee4db944412e5c87807c742d3c src/main/resources/org/apache/aurora/scheduler/http/ui/breadcrumb.html c780b0fe98863b5421824a9652a7202da9f4302a src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css 2a752313cb8ae404605a9458b33237a911eec061 src/main/resources/org/apache/aurora/scheduler/http/ui/job.html e21dee015897eee62ade8f74e26f042b8e2be734 src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js fb3b5b12121a6e8a30dbf6fe069557f69a563408 src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 3477b7e667459665af9d9dc9d2456793822bc7f7 src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js 7f05a552f3786adb115ff9648f4fadce968230e9 src/main/resources/org/apache/aurora/scheduler/http/ui/js/filters.js df2806481fc1c2f263d3afd9b21247e97a18ed57 src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js bfd360de45c75441743c8ba24a5c445f4146dba6 src/main/resources/org/apache/aurora/scheduler/http/ui/timeDisplay.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/update.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html PRE-CREATION src/main/thrift/org/apache/aurora/gen/api.thrift 2b376d663b3bc9264965db58ef89de59b10169ad Diff: https://reviews.apache.org/r/25259/diff/ Testing --- ./gradlew jsHint File Attachments job page https://reviews.apache.org/media/uploaded/files/2014/09/09/531eca81-a0ba-4438-8bd6-4b50d97b0270__job-progress-small-preview.png update page https://reviews.apache.org/media/uploaded/files/2014/09/09/8e3f2950-7d7e-404e-bca9-6c472b5218f7__update-page-finished.png Thanks, David McLaughlin
Re: Review Request 25259: Add update information to the scheduler UI
> On Sept. 12, 2014, 9:20 p.m., Joshua Cohen wrote:
> > src/main/resources/org/apache/aurora/scheduler/http/ui/js/filters.js, lines
> > 69-72
> > <https://reviews.apache.org/r/25259/diff/3/?file=684205#file684205line69>
> >
> > This might read a little bit cleaner if you chained it all?
> >
> > return instanceActionLookup[action] || 'UNKNOWN'
> > .replace(/INSTANCE_/, '')
> > .replace(/_/g, ' ');
Fixed.
> On Sept. 12, 2014, 9:20 p.m., Joshua Cohen wrote:
> > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js, line
> > 193
> > <https://reviews.apache.org/r/25259/diff/3/?file=684206#file684206line193>
> >
> > I'm unclear on the need to convert these arrays of statuses above to
> > objects just to check for the presence of a value? Is there a reason we
> > can't simply use indexOf on the array and save the transform?
It's definitely not worth it for status updates since even O(nm) is going to be
trivially small, but I think it's worthwhile* for the instance actions since
you're talking about potentially thousands of actions there. And once you've
got that toSet function, it's basically the same amount of code either way.
* Don't have a single benchmark to back this up, probably premature on
something like v8.
> On Sept. 12, 2014, 9:20 p.m., Joshua Cohen wrote:
> > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js, line
> > 246
> > <https://reviews.apache.org/r/25259/diff/3/?file=684206#file684206line246>
> >
> > just inline instanceActionLookup[action] here?
Fixed.
> On Sept. 12, 2014, 9:20 p.m., Joshua Cohen wrote:
> > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js,
> > lines 252-256
> > <https://reviews.apache.org/r/25259/diff/3/?file=684206#file684206line252>
> >
> > given the similarity to the reverse events iteration in
> > `progressFromEvents` above it might be worthwhile to extract a function
> > like (but with a better name...):
> >
> > function reverseEventsFilter(instanceEvents, condition) {
> > var events = _.sortBy(instanceEvents, 'timestampMs');
> > var instanceMap = {};
> > condition = condition || function() {};
> > for (var i = events.length - 1; i >= 0; i++) {
> > if (instanceMap.hasOwnProperty(events[i].instanceId) &&
> > condition(event)) {
> > instanceMap[event.instanceId] = event;
> > }
> > }
> >
> > return instanceMap;
> > }
> >
> > Then the logic here just becomes:
> >
> > var instanceMap = reverseEventsFilter(details.instanceEvents);
> >
> > And the logic above in `progressFromEvents` becomes something like:
> >
> > return Object.keys(reverseEventsFilter(instanceEvents, function(e)
> > { updateUtil.isInstanceSuccessful(e.action); })).length;
Yeah, I had repeated this code at least one more time when I had the event
timeline viz too. I like this solution.
Fixed.
> On Sept. 12, 2014, 9:20 p.m., Joshua Cohen wrote:
> > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js, line
> > 268
> > <https://reviews.apache.org/r/25259/diff/3/?file=684206#file684206line268>
> >
> > kill blank line.
Fixed.
> On Sept. 12, 2014, 9:20 p.m., Joshua Cohen wrote:
> > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js, line
> > 289
> > <https://reviews.apache.org/r/25259/diff/3/?file=684206#file684206line289>
> >
> > isSubset checks to make sure subset is truthy, and since it's iterating
> > to subset.length we can probably skip both of these checks and just make
> > this `if (inSubset(i))`
> >
> > Also, how do you feel about passing subset in as a param to isSubset
> > instead of picking it up via a closure?
+1 for making subset a param and removing the length check.
But the other suggestion breaks the logic. It really is "if they have declared
a subset AND this instance id isn't in that subset.. it's not going to be part
of the update" (i.e. ignored). You can rejig the else if and else but you
always have to check both conditions. I thought you were right at first, which
speaks to the clarity of the code here... so I added a comment to clarify.
(And also speaks for how badly we need unit tests!)
- David
-
Re: Review Request 25259: Add update information to the scheduler UI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/ --- (Updated Sept. 12, 2014, 10:39 p.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner. Changes --- Forgot to run jshint + forgot to refactor one of the event parsing functions. Bugs: AURORA-614 https://issues.apache.org/jira/browse/AURORA-614 Repository: aurora Description --- Adds update history to the job page. Adds an update details page. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java de49a1c5497e32ee4db944412e5c87807c742d3c src/main/resources/org/apache/aurora/scheduler/http/ui/breadcrumb.html c780b0fe98863b5421824a9652a7202da9f4302a src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css 2a752313cb8ae404605a9458b33237a911eec061 src/main/resources/org/apache/aurora/scheduler/http/ui/job.html e21dee015897eee62ade8f74e26f042b8e2be734 src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js fb3b5b12121a6e8a30dbf6fe069557f69a563408 src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 3477b7e667459665af9d9dc9d2456793822bc7f7 src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js 7f05a552f3786adb115ff9648f4fadce968230e9 src/main/resources/org/apache/aurora/scheduler/http/ui/js/filters.js df2806481fc1c2f263d3afd9b21247e97a18ed57 src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js bfd360de45c75441743c8ba24a5c445f4146dba6 src/main/resources/org/apache/aurora/scheduler/http/ui/timeDisplay.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/update.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html PRE-CREATION src/main/thrift/org/apache/aurora/gen/api.thrift 2b376d663b3bc9264965db58ef89de59b10169ad Diff: https://reviews.apache.org/r/25259/diff/ Testing --- ./gradlew jsHint File Attachments job page https://reviews.apache.org/media/uploaded/files/2014/09/09/531eca81-a0ba-4438-8bd6-4b50d97b0270__job-progress-small-preview.png update page https://reviews.apache.org/media/uploaded/files/2014/09/09/8e3f2950-7d7e-404e-bca9-6c472b5218f7__update-page-finished.png Thanks, David McLaughlin
Re: Review Request 25259: Add update information to the scheduler UI
> On Sept. 12, 2014, 11:47 p.m., Bill Farner wrote: > > src/main/resources/org/apache/aurora/scheduler/http/ui/js/filters.js, line > > 54 > > <https://reviews.apache.org/r/25259/diff/5/?file=688116#file688116line54> > > > > I'd like to see the paused states disambiguated. I could imagine a > > user resuming an update and being surprised by the direction the update > > moves. Fixed. > On Sept. 12, 2014, 11:47 p.m., Bill Farner wrote: > > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js, line > > 338 > > <https://reviews.apache.org/r/25259/diff/5/?file=688117#file688117line338> > > > > TODO(davmclau) Dropped this from the UI, so just removed this entirely. > On Sept. 12, 2014, 11:47 p.m., Bill Farner wrote: > > src/main/resources/org/apache/aurora/scheduler/http/ui/timeDisplay.html, > > line 17 > > <https://reviews.apache.org/r/25259/diff/5/?file=688118#file688118line17> > > > > I'd love to see UTC here as well. I added it as a tooltip, so when you mouseover the time it will show you the UTC version. Do you think that's good enough or would you rather it just be visible at all times? > On Sept. 12, 2014, 11:47 p.m., Bill Farner wrote: > > src/main/resources/org/apache/aurora/scheduler/http/ui/update.html, line 24 > > <https://reviews.apache.org/r/25259/diff/5/?file=688119#file688119line24> > > > > Is the indenting off here? Fixed. Unfortunately my text editor does not play well with custom angular directives so I have to maintain this all manually. > On Sept. 12, 2014, 11:47 p.m., Bill Farner wrote: > > src/main/resources/org/apache/aurora/scheduler/http/ui/update.html, line 129 > > <https://reviews.apache.org/r/25259/diff/5/?file=688119#file688119line129> > > > > For my own edification - is there a pattern being followed w.r.t. > > newlines here? I thought i had the style figured out until here. Not really, at this point it's more of an art form. This particular example (and the indentation problem above) was a result of me removing the event timeline from the DOM and not going back and fixing everything. Fixed it now. Happy to open a ticket to investigate HTML linting if there is sufficient interest. - David ------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/#review53244 --- On Sept. 12, 2014, 10:39 p.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25259/ > --- > > (Updated Sept. 12, 2014, 10:39 p.m.) > > > Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner. > > > Bugs: AURORA-614 > https://issues.apache.org/jira/browse/AURORA-614 > > > Repository: aurora > > > Description > --- > > Adds update history to the job page. Adds an update details page. > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java > de49a1c5497e32ee4db944412e5c87807c742d3c > src/main/resources/org/apache/aurora/scheduler/http/ui/breadcrumb.html > c780b0fe98863b5421824a9652a7202da9f4302a > src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css > 2a752313cb8ae404605a9458b33237a911eec061 > src/main/resources/org/apache/aurora/scheduler/http/ui/job.html > e21dee015897eee62ade8f74e26f042b8e2be734 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js > fb3b5b12121a6e8a30dbf6fe069557f69a563408 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js > 3477b7e667459665af9d9dc9d2456793822bc7f7 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js > 7f05a552f3786adb115ff9648f4fadce968230e9 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/filters.js > df2806481fc1c2f263d3afd9b21247e97a18ed57 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js > bfd360de45c75441743c8ba24a5c445f4146dba6 > src/main/resources/org/apache/aurora/scheduler/http/ui/timeDisplay.html > PRE-CREATION > src/main/resources/org/apache/aurora/scheduler/http/ui/update.html > PRE-CREATION > src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html > PRE-CREATION > src/main/thrift/org/apache/aurora/gen/api.thrift > 2b376d663b3bc9264965db58ef89de59b10169ad > > Diff: https://reviews.apache.org/r/25259/diff/ > > > Test
Re: Review Request 25259: Add update information to the scheduler UI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/ --- (Updated Sept. 15, 2014, 6:53 p.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner. Bugs: AURORA-614 https://issues.apache.org/jira/browse/AURORA-614 Repository: aurora Description --- Adds update history to the job page. Adds an update details page. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java de49a1c5497e32ee4db944412e5c87807c742d3c src/main/resources/org/apache/aurora/scheduler/http/ui/breadcrumb.html c780b0fe98863b5421824a9652a7202da9f4302a src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css 2a752313cb8ae404605a9458b33237a911eec061 src/main/resources/org/apache/aurora/scheduler/http/ui/job.html e21dee015897eee62ade8f74e26f042b8e2be734 src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js fb3b5b12121a6e8a30dbf6fe069557f69a563408 src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 3477b7e667459665af9d9dc9d2456793822bc7f7 src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js 7f05a552f3786adb115ff9648f4fadce968230e9 src/main/resources/org/apache/aurora/scheduler/http/ui/js/filters.js df2806481fc1c2f263d3afd9b21247e97a18ed57 src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js bfd360de45c75441743c8ba24a5c445f4146dba6 src/main/resources/org/apache/aurora/scheduler/http/ui/timeDisplay.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/update.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html PRE-CREATION src/main/thrift/org/apache/aurora/gen/api.thrift 2b376d663b3bc9264965db58ef89de59b10169ad Diff: https://reviews.apache.org/r/25259/diff/ Testing --- ./gradlew jsHint File Attachments job page https://reviews.apache.org/media/uploaded/files/2014/09/09/531eca81-a0ba-4438-8bd6-4b50d97b0270__job-progress-small-preview.png update page https://reviews.apache.org/media/uploaded/files/2014/09/09/8e3f2950-7d7e-404e-bca9-6c472b5218f7__update-page-finished.png Thanks, David McLaughlin
Re: Review Request 25259: Add update information to the scheduler UI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/ --- (Updated Sept. 16, 2014, 12:30 a.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner. Changes --- rebase. Bugs: AURORA-614 https://issues.apache.org/jira/browse/AURORA-614 Repository: aurora Description --- Adds update history to the job page. Adds an update details page. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 0336a6e00a9a7e14125eaae271788faaae0bd371 src/main/resources/org/apache/aurora/scheduler/http/ui/breadcrumb.html c780b0fe98863b5421824a9652a7202da9f4302a src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css 2a752313cb8ae404605a9458b33237a911eec061 src/main/resources/org/apache/aurora/scheduler/http/ui/job.html e21dee015897eee62ade8f74e26f042b8e2be734 src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js fb3b5b12121a6e8a30dbf6fe069557f69a563408 src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 3477b7e667459665af9d9dc9d2456793822bc7f7 src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js 7f05a552f3786adb115ff9648f4fadce968230e9 src/main/resources/org/apache/aurora/scheduler/http/ui/js/filters.js 76f48f8aa2399c83fbafe8efcf6f4f3e47ab05a0 src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js e898f12a7b6c6c35b4bce5a8aa78f3fcaa83a6df src/main/resources/org/apache/aurora/scheduler/http/ui/timeDisplay.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/update.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html PRE-CREATION src/main/thrift/org/apache/aurora/gen/api.thrift a6dc548c804bfcb9166573496023bad80b2a2c91 Diff: https://reviews.apache.org/r/25259/diff/ Testing --- ./gradlew jsHint File Attachments job page https://reviews.apache.org/media/uploaded/files/2014/09/09/531eca81-a0ba-4438-8bd6-4b50d97b0270__job-progress-small-preview.png update page https://reviews.apache.org/media/uploaded/files/2014/09/09/8e3f2950-7d7e-404e-bca9-6c472b5218f7__update-page-finished.png Thanks, David McLaughlin
Re: Review Request 25259: Add update information to the scheduler UI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/ --- (Updated Sept. 16, 2014, 12:32 a.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner. Changes --- Rebase went slightly wrong due to the mocked out data commits I have. Fixed. Bugs: AURORA-614 https://issues.apache.org/jira/browse/AURORA-614 Repository: aurora Description --- Adds update history to the job page. Adds an update details page. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 0336a6e00a9a7e14125eaae271788faaae0bd371 src/main/resources/org/apache/aurora/scheduler/http/ui/breadcrumb.html c780b0fe98863b5421824a9652a7202da9f4302a src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css 2a752313cb8ae404605a9458b33237a911eec061 src/main/resources/org/apache/aurora/scheduler/http/ui/job.html e21dee015897eee62ade8f74e26f042b8e2be734 src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js fb3b5b12121a6e8a30dbf6fe069557f69a563408 src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 3477b7e667459665af9d9dc9d2456793822bc7f7 src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js 7f05a552f3786adb115ff9648f4fadce968230e9 src/main/resources/org/apache/aurora/scheduler/http/ui/js/filters.js 76f48f8aa2399c83fbafe8efcf6f4f3e47ab05a0 src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js e898f12a7b6c6c35b4bce5a8aa78f3fcaa83a6df src/main/resources/org/apache/aurora/scheduler/http/ui/timeDisplay.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/update.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html PRE-CREATION src/main/thrift/org/apache/aurora/gen/api.thrift a6dc548c804bfcb9166573496023bad80b2a2c91 Diff: https://reviews.apache.org/r/25259/diff/ Testing --- ./gradlew jsHint File Attachments job page https://reviews.apache.org/media/uploaded/files/2014/09/09/531eca81-a0ba-4438-8bd6-4b50d97b0270__job-progress-small-preview.png update page https://reviews.apache.org/media/uploaded/files/2014/09/09/8e3f2950-7d7e-404e-bca9-6c472b5218f7__update-page-finished.png Thanks, David McLaughlin
Re: Review Request 25259: Add update information to the scheduler UI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/ --- (Updated Sept. 16, 2014, 6:06 p.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner. Changes --- Hide the latest updates content box entirely when there are no updates. Bugs: AURORA-614 https://issues.apache.org/jira/browse/AURORA-614 Repository: aurora Description --- Adds update history to the job page. Adds an update details page. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 0336a6e00a9a7e14125eaae271788faaae0bd371 src/main/resources/org/apache/aurora/scheduler/http/ui/breadcrumb.html c780b0fe98863b5421824a9652a7202da9f4302a src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css 2a752313cb8ae404605a9458b33237a911eec061 src/main/resources/org/apache/aurora/scheduler/http/ui/job.html e21dee015897eee62ade8f74e26f042b8e2be734 src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js fb3b5b12121a6e8a30dbf6fe069557f69a563408 src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 3477b7e667459665af9d9dc9d2456793822bc7f7 src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js 7f05a552f3786adb115ff9648f4fadce968230e9 src/main/resources/org/apache/aurora/scheduler/http/ui/js/filters.js 76f48f8aa2399c83fbafe8efcf6f4f3e47ab05a0 src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js e898f12a7b6c6c35b4bce5a8aa78f3fcaa83a6df src/main/resources/org/apache/aurora/scheduler/http/ui/timeDisplay.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/update.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html PRE-CREATION src/main/thrift/org/apache/aurora/gen/api.thrift a6dc548c804bfcb9166573496023bad80b2a2c91 Diff: https://reviews.apache.org/r/25259/diff/ Testing --- ./gradlew jsHint File Attachments job page https://reviews.apache.org/media/uploaded/files/2014/09/09/531eca81-a0ba-4438-8bd6-4b50d97b0270__job-progress-small-preview.png update page https://reviews.apache.org/media/uploaded/files/2014/09/09/8e3f2950-7d7e-404e-bca9-6c472b5218f7__update-page-finished.png Thanks, David McLaughlin
Re: Review Request 25259: Add update information to the scheduler UI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/ --- (Updated Sept. 16, 2014, 6:29 p.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner. Changes --- Modified the maxFailedInstances description. Bugs: AURORA-614 https://issues.apache.org/jira/browse/AURORA-614 Repository: aurora Description --- Adds update history to the job page. Adds an update details page. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 0336a6e00a9a7e14125eaae271788faaae0bd371 src/main/resources/org/apache/aurora/scheduler/http/ui/breadcrumb.html c780b0fe98863b5421824a9652a7202da9f4302a src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css 2a752313cb8ae404605a9458b33237a911eec061 src/main/resources/org/apache/aurora/scheduler/http/ui/job.html e21dee015897eee62ade8f74e26f042b8e2be734 src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js fb3b5b12121a6e8a30dbf6fe069557f69a563408 src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 3477b7e667459665af9d9dc9d2456793822bc7f7 src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js 7f05a552f3786adb115ff9648f4fadce968230e9 src/main/resources/org/apache/aurora/scheduler/http/ui/js/filters.js 76f48f8aa2399c83fbafe8efcf6f4f3e47ab05a0 src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js e898f12a7b6c6c35b4bce5a8aa78f3fcaa83a6df src/main/resources/org/apache/aurora/scheduler/http/ui/timeDisplay.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/update.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html PRE-CREATION src/main/thrift/org/apache/aurora/gen/api.thrift a6dc548c804bfcb9166573496023bad80b2a2c91 Diff: https://reviews.apache.org/r/25259/diff/ Testing --- ./gradlew jsHint File Attachments job page https://reviews.apache.org/media/uploaded/files/2014/09/09/531eca81-a0ba-4438-8bd6-4b50d97b0270__job-progress-small-preview.png update page https://reviews.apache.org/media/uploaded/files/2014/09/09/8e3f2950-7d7e-404e-bca9-6c472b5218f7__update-page-finished.png Thanks, David McLaughlin
Re: Review Request 25259: Add update information to the scheduler UI
> On Sept. 16, 2014, 6:12 p.m., Maxim Khutornenko wrote: > > src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html, > > line 41 > > <https://reviews.apache.org/r/25259/diff/9/?file=690739#file690739line41> > > > > "...forward roll." - this is no longer correct as we use this setting > > for both forward and back roll. Mind updating this and api.thrift? Done. Can you confirm you're okay with the updated description? - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/#review53555 ------- On Sept. 16, 2014, 6:29 p.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25259/ > --- > > (Updated Sept. 16, 2014, 6:29 p.m.) > > > Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner. > > > Bugs: AURORA-614 > https://issues.apache.org/jira/browse/AURORA-614 > > > Repository: aurora > > > Description > --- > > Adds update history to the job page. Adds an update details page. > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java > 0336a6e00a9a7e14125eaae271788faaae0bd371 > src/main/resources/org/apache/aurora/scheduler/http/ui/breadcrumb.html > c780b0fe98863b5421824a9652a7202da9f4302a > src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css > 2a752313cb8ae404605a9458b33237a911eec061 > src/main/resources/org/apache/aurora/scheduler/http/ui/job.html > e21dee015897eee62ade8f74e26f042b8e2be734 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js > fb3b5b12121a6e8a30dbf6fe069557f69a563408 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js > 3477b7e667459665af9d9dc9d2456793822bc7f7 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js > 7f05a552f3786adb115ff9648f4fadce968230e9 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/filters.js > 76f48f8aa2399c83fbafe8efcf6f4f3e47ab05a0 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js > e898f12a7b6c6c35b4bce5a8aa78f3fcaa83a6df > src/main/resources/org/apache/aurora/scheduler/http/ui/timeDisplay.html > PRE-CREATION > src/main/resources/org/apache/aurora/scheduler/http/ui/update.html > PRE-CREATION > src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html > PRE-CREATION > src/main/thrift/org/apache/aurora/gen/api.thrift > a6dc548c804bfcb9166573496023bad80b2a2c91 > > Diff: https://reviews.apache.org/r/25259/diff/ > > > Testing > --- > > ./gradlew jsHint > > > File Attachments > > > job page > > https://reviews.apache.org/media/uploaded/files/2014/09/09/531eca81-a0ba-4438-8bd6-4b50d97b0270__job-progress-small-preview.png > update page > > https://reviews.apache.org/media/uploaded/files/2014/09/09/8e3f2950-7d7e-404e-bca9-6c472b5218f7__update-page-finished.png > > > Thanks, > > David McLaughlin > >
Re: Review Request 25710: Remove the JobUpdateAction.INSTANCE_SKIPPED.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25710/#review53585 --- services.js in the commit I shipped today has two references to INSTANCES_SKIPPED. - David McLaughlin On Sept. 16, 2014, 8:37 p.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25710/ > --- > > (Updated Sept. 16, 2014, 8:37 p.m.) > > > Review request for Aurora and David McLaughlin. > > > Repository: aurora > > > Description > --- > > Didn't see any reference in the UI code yet, so looks like this is a > thrift-only change. > > > Diffs > - > > src/main/thrift/org/apache/aurora/gen/api.thrift > a6dc548c804bfcb9166573496023bad80b2a2c91 > > Diff: https://reviews.apache.org/r/25710/diff/ > > > Testing > --- > > ./gradlew build -Pq > > > Thanks, > > Bill Farner > >
Re: Review Request 25710: Remove the JobUpdateAction.INSTANCE_SKIPPED.
> On Sept. 16, 2014, 8:49 p.m., David McLaughlin wrote: > > services.js in the commit I shipped today has two references to > > INSTANCES_SKIPPED. > > Bill Farner wrote: > Aha, you saw the writing on the wall and snuck in :-P :) I'm fine with you just punting on changing them and leaving them for later when we add the second part of this... but they are also really easy references to remove. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25710/#review53585 --- On Sept. 16, 2014, 8:55 p.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25710/ > --- > > (Updated Sept. 16, 2014, 8:55 p.m.) > > > Review request for Aurora and David McLaughlin. > > > Repository: aurora > > > Description > --- > > Didn't see any reference in the UI code yet, so looks like this is a > thrift-only change. > > > Diffs > - > > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js > c80146aa3829e3c3102645a1864dbeaf5e2e56bc > src/main/thrift/org/apache/aurora/gen/api.thrift > e7770e651596a58b39138aadc240c45aaeb4230a > > Diff: https://reviews.apache.org/r/25710/diff/ > > > Testing > --- > > ./gradlew build -Pq > > > Thanks, > > Bill Farner > >
Re: Review Request 25710: Remove the JobUpdateAction.INSTANCE_SKIPPED.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25710/#review53592 --- Ship it! Ship It! - David McLaughlin On Sept. 16, 2014, 8:55 p.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25710/ > --- > > (Updated Sept. 16, 2014, 8:55 p.m.) > > > Review request for Aurora and David McLaughlin. > > > Repository: aurora > > > Description > --- > > Didn't see any reference in the UI code yet, so looks like this is a > thrift-only change. > > > Diffs > - > > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js > c80146aa3829e3c3102645a1864dbeaf5e2e56bc > src/main/thrift/org/apache/aurora/gen/api.thrift > e7770e651596a58b39138aadc240c45aaeb4230a > > Diff: https://reviews.apache.org/r/25710/diff/ > > > Testing > --- > > ./gradlew build -Pq > > > Thanks, > > Bill Farner > >
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
> 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 25741: Remove INSTANCE_ADDED and INSTANCE_REMOVED states.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25741/#review53694 --- Does removing an instance take any significant amount of time? Can it fail? The concern is we'll be showing 100% progress but the update is still running when reducing the instance count. - David McLaughlin On Sept. 17, 2014, 5:15 p.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25741/ > --- > > (Updated Sept. 17, 2014, 5:15 p.m.) > > > Review request for Aurora, David McLaughlin and Maxim Khutornenko. > > > Repository: aurora > > > Description > --- > > Since we're changing fields in `JobUpdateConfiguration` (being renamed to > `JobUpdateInstructions`) to represent the delta incurred by a job update > (AURORA-718), i believe we can remove these two states, as addition/removal > can be inferred from the delta. > > Furthermore, this allows the job updater's internal state machine to > translate directly into the remaining `JobUpdateActions`, and hides details > of the non-atomic actions necessary to move an instance from state A to state > B. > > > Diffs > - > > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js > 5d6299f9de6eccd0f1332e11d57dfb910d956011 > src/main/thrift/org/apache/aurora/gen/api.thrift > 3d0beeaed74aafcec0e24725f443e53a67f6c3a0 > > src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java > e09caa63bc0150d7109cb237e80b9efee441dded > src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java > ca990e73d80e8456e71a97f0bdd0b6f4530d0135 > > Diff: https://reviews.apache.org/r/25741/diff/ > > > Testing > --- > > ./gradlew build -Pq > > > Thanks, > > Bill Farner > >
Re: Review Request 25721: Asynchronous JS for Scheduler UI
--- 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
Re: Review Request 25721: Asynchronous JS for Scheduler UI
> 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 25741: Remove INSTANCE_ADDED and INSTANCE_REMOVED states.
> On Sept. 17, 2014, 5:21 p.m., David McLaughlin wrote: > > Does removing an instance take any significant amount of time? Can it fail? > > The concern is we'll be showing 100% progress but the update is still > > running when reducing the instance count. > > Bill Farner wrote: > Depends on your definition of significant. It does take a > non-deterministic amount of time, since we transition to KILLING and await to > see that the task is gone. It can _sort of_ fail, but not in a way that we > will record an instance failure AFAICT. However, i think the behavior of > ACTING -> FINISHED even in removal is nice as it matches two-step nature of > the operation. I see, so the new behaviour is that in both scenarios the instance would end up in INSTANCE_UPDATED state? And then the client infers whether it was added or removed based on the instance count delta? - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25741/#review53694 --- On Sept. 17, 2014, 5:15 p.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25741/ > --- > > (Updated Sept. 17, 2014, 5:15 p.m.) > > > Review request for Aurora, David McLaughlin and Maxim Khutornenko. > > > Repository: aurora > > > Description > --- > > Since we're changing fields in `JobUpdateConfiguration` (being renamed to > `JobUpdateInstructions`) to represent the delta incurred by a job update > (AURORA-718), i believe we can remove these two states, as addition/removal > can be inferred from the delta. > > Furthermore, this allows the job updater's internal state machine to > translate directly into the remaining `JobUpdateActions`, and hides details > of the non-atomic actions necessary to move an instance from state A to state > B. > > > Diffs > - > > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js > 5d6299f9de6eccd0f1332e11d57dfb910d956011 > src/main/thrift/org/apache/aurora/gen/api.thrift > 3d0beeaed74aafcec0e24725f443e53a67f6c3a0 > > src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java > e09caa63bc0150d7109cb237e80b9efee441dded > src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java > ca990e73d80e8456e71a97f0bdd0b6f4530d0135 > > Diff: https://reviews.apache.org/r/25741/diff/ > > > Testing > --- > > ./gradlew build -Pq > > > Thanks, > > Bill Farner > >
Re: Review Request 25741: Remove INSTANCE_ADDED and INSTANCE_REMOVED states.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25741/#review53706 --- Ship it! Ship It! - David McLaughlin On Sept. 17, 2014, 5:15 p.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25741/ > --- > > (Updated Sept. 17, 2014, 5:15 p.m.) > > > Review request for Aurora, David McLaughlin and Maxim Khutornenko. > > > Repository: aurora > > > Description > --- > > Since we're changing fields in `JobUpdateConfiguration` (being renamed to > `JobUpdateInstructions`) to represent the delta incurred by a job update > (AURORA-718), i believe we can remove these two states, as addition/removal > can be inferred from the delta. > > Furthermore, this allows the job updater's internal state machine to > translate directly into the remaining `JobUpdateActions`, and hides details > of the non-atomic actions necessary to move an instance from state A to state > B. > > > Diffs > - > > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js > 5d6299f9de6eccd0f1332e11d57dfb910d956011 > src/main/thrift/org/apache/aurora/gen/api.thrift > 3d0beeaed74aafcec0e24725f443e53a67f6c3a0 > > src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java > e09caa63bc0150d7109cb237e80b9efee441dded > src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java > ca990e73d80e8456e71a97f0bdd0b6f4530d0135 > > Diff: https://reviews.apache.org/r/25741/diff/ > > > Testing > --- > > ./gradlew build -Pq > > > Thanks, > > Bill Farner > >
Re: Review Request 25750: Store new task configuration in JobUpdateConfiguration as InstanceTaskConfig
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25750/#review53746 --- +1 to the code change, UI stuff in particular looks good to me. -1 to dropping instanceCount completely though. I thought we mentioned we wanted to capture and store all the original details the user sends? Even if this is purely for auditing and never used internally, I still think it's useful. - David McLaughlin On Sept. 17, 2014, 8:40 p.m., Maxim Khutornenko wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25750/ > --- > > (Updated Sept. 17, 2014, 8:40 p.m.) > > > Review request for Aurora, David McLaughlin and Bill Farner. > > > Bugs: AURORA-717 > https://issues.apache.org/jira/browse/AURORA-717 > > > Repository: aurora > > > Description > --- > > Converted newTaskConfig into InstanceTaskConfig to allow multiple instance > ranges. > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java > 5ac42b6860a1e99f27b6a4067d370f26943f9212 > src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java > dfaadc8cf6bf1d929e4e5fec8347a804c6478122 > src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java > 0737f92bc3ea971c64ca6d84b02da7e2b5b934bf > > src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java > 04a9246467ce140300b3b543bdb98ad4fe8302ff > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java > d4b8141d9d483a21d18afd9c6fbb2cf639595101 > > src/main/java/org/apache/aurora/scheduler/updater/InstanceActionHandler.java > 25382910704f86e6ca292c7f8eae5990663c4b46 > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java > 159b09e7c00175bf3aea893d48cb3953186bd6cb > src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java > 82f2b6d98e8270efb9b6517b1f9782a8c5a9aa39 > 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 > 5d6299f9de6eccd0f1332e11d57dfb910d956011 > src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html > 613b5325a3ae53fa61e6bac58bcc6e76950f7031 > > src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml > 0e7f6ebd35a6dee28a16e28fa7b10b10d20c70c5 > src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql > a450a090f76fe565924e2f9c5340c10d1f6f05be > src/main/thrift/org/apache/aurora/gen/api.thrift > 3d0beeaed74aafcec0e24725f443e53a67f6c3a0 > > src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java > e09caa63bc0150d7109cb237e80b9efee441dded > src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java > ca990e73d80e8456e71a97f0bdd0b6f4530d0135 > > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java > 7d4dd3720ed946c1dd10b9f3979ded796fb15d98 > src/test/java/org/apache/aurora/scheduler/updater/AddTaskTest.java > 1b8e5c2c9e21810589b6770129f742de4f1a67e2 > src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java > 0e15a79ae6dc877dfa6ba492efafdf1f89f7d40f > > src/test/java/org/apache/aurora/scheduler/updater/UpdateFactoryImplTest.java > f698b532a3827e59e654d6e07e20f5725aed6768 > src/test/resources/org/apache/aurora/gen/api.thrift.md5 > c5838761783d85a547688d4f708a75c1fd240201 > > Diff: https://reviews.apache.org/r/25750/diff/ > > > Testing > --- > > gradle -Pq build > ./pants src/test/python:all > > > Thanks, > > Maxim Khutornenko > >
Re: Review Request 25753: Surface instance update status changes so they may be persisted.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25753/#review53753 --- Ship it! Other than the docs, lgtm. - David McLaughlin On Sept. 17, 2014, 8:55 p.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25753/ > --- > > (Updated Sept. 17, 2014, 8:55 p.m.) > > > Review request for Aurora, David McLaughlin and Maxim Khutornenko. > > > Repository: aurora > > > Description > --- > > Rework outputs from OneWayJobUpdater so JobUpdateControllerImpl has > visibility into internal state machine transitions, which is the last bit > necessary before we can start saving instance update events. > > Also contains a small bit of cleanup: > - removed TODOs that have been addressed > - made Optional a member of Result, rather than having a > switch to map between them. > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java > dc11abda72855f925a93913f57c5d0e15bc2e0ff > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java > 159b09e7c00175bf3aea893d48cb3953186bd6cb > src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java > 5a9af39ca619b90dfbac16b8fed55a3f7127cce3 > src/main/java/org/apache/aurora/scheduler/updater/SideEffect.java > PRE-CREATION > src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java > 453daeb0b1bb6c9d958e0265d0b7379e1b7a6558 > src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java > 82f2b6d98e8270efb9b6517b1f9782a8c5a9aa39 > src/test/java/org/apache/aurora/scheduler/updater/EnumsTest.java > defdf6ea056b6524a8fc25f76be37f9ed0fad3e8 > src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java > 6169471388eabc83245fbe3b8abb2c3a38eb00e1 > > Diff: https://reviews.apache.org/r/25753/diff/ > > > Testing > --- > > ./gradlew build -Pq > > > Thanks, > > Bill Farner > >
Re: Review Request 25750: Store new task configuration in JobUpdateConfiguration as InstanceTaskConfig
> On Sept. 17, 2014, 9:18 p.m., David McLaughlin wrote: > > +1 to the code change, UI stuff in particular looks good to me. > > > > -1 to dropping instanceCount completely though. I thought we mentioned we > > wanted to capture and store all the original details the user sends? Even > > if this is purely for auditing and never used internally, I still think > > it's useful. > > Bill Farner wrote: > Should we change direction a bit and just store the original > JobUpdateRequest? > > Maxim Khutornenko wrote: > Storing instance count is completely redundant and prone to integrity > problems (i.e. need to make sure instance ranges match instance counts). > > Maxim Khutornenko wrote: > | Should we change direction a bit and just store the original > JobUpdateRequest > Don't really see what it would buy us. All that data is already available > in the current schema. Instance count is not instanceRanges.length, instead it's an option the user sent as part of the update - the value we were already storing. It's redundant once the instance ranges have been calculated but it is used to make that calculation the first time (in the absence of updateOnlyTheseInstances). I think it's useful for auditing and figuring out why the scheduler made the decision it did. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25750/#review53746 --- On Sept. 17, 2014, 8:40 p.m., Maxim Khutornenko wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25750/ > ------- > > (Updated Sept. 17, 2014, 8:40 p.m.) > > > Review request for Aurora, David McLaughlin and Bill Farner. > > > Bugs: AURORA-717 > https://issues.apache.org/jira/browse/AURORA-717 > > > Repository: aurora > > > Description > --- > > Converted newTaskConfig into InstanceTaskConfig to allow multiple instance > ranges. > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java > 5ac42b6860a1e99f27b6a4067d370f26943f9212 > src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java > dfaadc8cf6bf1d929e4e5fec8347a804c6478122 > src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java > 0737f92bc3ea971c64ca6d84b02da7e2b5b934bf > > src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java > 04a9246467ce140300b3b543bdb98ad4fe8302ff > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java > d4b8141d9d483a21d18afd9c6fbb2cf639595101 > > src/main/java/org/apache/aurora/scheduler/updater/InstanceActionHandler.java > 25382910704f86e6ca292c7f8eae5990663c4b46 > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java > 159b09e7c00175bf3aea893d48cb3953186bd6cb > src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java > 82f2b6d98e8270efb9b6517b1f9782a8c5a9aa39 > 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 > 5d6299f9de6eccd0f1332e11d57dfb910d956011 > src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html > 613b5325a3ae53fa61e6bac58bcc6e76950f7031 > > src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml > 0e7f6ebd35a6dee28a16e28fa7b10b10d20c70c5 > src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql > a450a090f76fe565924e2f9c5340c10d1f6f05be > src/main/thrift/org/apache/aurora/gen/api.thrift > 3d0beeaed74aafcec0e24725f443e53a67f6c3a0 > > src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java > e09caa63bc0150d7109cb237e80b9efee441dded > src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java > ca990e73d80e8456e71a97f0bdd0b6f4530d0135 > > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java > 7d4dd3720ed946c1dd10b9f3979ded796fb15d98 > src/test/java/org/apache/aurora/scheduler/updater/AddTaskTest.java > 1b8e5c2c9e21810589b6770129f742de4f1a67e2 > src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java > 0e15a79ae6dc877dfa6ba492efafdf1f89f7d40f > > src/test/java/org/apache/aurora/scheduler/updater/UpdateFactoryImplTest.java > f698b532a3827e59e654d6e07e20f5725aed6768 > src/test/resources/org/apache/aurora/gen/api.thrift.md5 > c5838761783d85a547688d4f708a75c1fd240201 > > Diff: https://reviews.apache.org/r/25750/diff/ > > > Testing > --- > > gradle -Pq build > ./pants src/test/python:all > > > Thanks, > > Maxim Khutornenko > >
Re: Review Request 25750: Store new task configuration in JobUpdateConfiguration as InstanceTaskConfig
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25750/#review53860 --- Ship it! Ship It! - David McLaughlin On Sept. 17, 2014, 8:40 p.m., Maxim Khutornenko wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25750/ > --- > > (Updated Sept. 17, 2014, 8:40 p.m.) > > > Review request for Aurora, David McLaughlin and Bill Farner. > > > Bugs: AURORA-717 > https://issues.apache.org/jira/browse/AURORA-717 > > > Repository: aurora > > > Description > --- > > Converted newTaskConfig into InstanceTaskConfig to allow multiple instance > ranges. > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java > 5ac42b6860a1e99f27b6a4067d370f26943f9212 > src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java > dfaadc8cf6bf1d929e4e5fec8347a804c6478122 > src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java > 0737f92bc3ea971c64ca6d84b02da7e2b5b934bf > > src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java > 04a9246467ce140300b3b543bdb98ad4fe8302ff > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java > d4b8141d9d483a21d18afd9c6fbb2cf639595101 > > src/main/java/org/apache/aurora/scheduler/updater/InstanceActionHandler.java > 25382910704f86e6ca292c7f8eae5990663c4b46 > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java > 159b09e7c00175bf3aea893d48cb3953186bd6cb > src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java > 82f2b6d98e8270efb9b6517b1f9782a8c5a9aa39 > 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 > 5d6299f9de6eccd0f1332e11d57dfb910d956011 > src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html > 613b5325a3ae53fa61e6bac58bcc6e76950f7031 > > src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml > 0e7f6ebd35a6dee28a16e28fa7b10b10d20c70c5 > src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql > a450a090f76fe565924e2f9c5340c10d1f6f05be > src/main/thrift/org/apache/aurora/gen/api.thrift > 3d0beeaed74aafcec0e24725f443e53a67f6c3a0 > > src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java > e09caa63bc0150d7109cb237e80b9efee441dded > src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java > ca990e73d80e8456e71a97f0bdd0b6f4530d0135 > > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java > 7d4dd3720ed946c1dd10b9f3979ded796fb15d98 > src/test/java/org/apache/aurora/scheduler/updater/AddTaskTest.java > 1b8e5c2c9e21810589b6770129f742de4f1a67e2 > src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java > 0e15a79ae6dc877dfa6ba492efafdf1f89f7d40f > > src/test/java/org/apache/aurora/scheduler/updater/UpdateFactoryImplTest.java > f698b532a3827e59e654d6e07e20f5725aed6768 > src/test/resources/org/apache/aurora/gen/api.thrift.md5 > c5838761783d85a547688d4f708a75c1fd240201 > > Diff: https://reviews.apache.org/r/25750/diff/ > > > Testing > --- > > gradle -Pq build > ./pants src/test/python:all > > > Thanks, > > Maxim Khutornenko > >
Re: Review Request 25791: Save instance update events when updating a job.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25791/#review53873 --- Ship it! Ship It! - David McLaughlin On Sept. 18, 2014, 8:54 p.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25791/ > --- > > (Updated Sept. 18, 2014, 8:54 p.m.) > > > Review request for Aurora, David McLaughlin and Maxim Khutornenko. > > > Bugs: AURORA-613 > https://issues.apache.org/jira/browse/AURORA-613 > > > Repository: aurora > > > Description > --- > > A follow-up change will scope the updaters to actionable instance changes, > which will remove the redundant actions being saved (see TODOs). > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/http/RequestLogger.java > 0442191d3a84e1a6bbeb2a79fda78ef077ee0d04 > src/main/java/org/apache/aurora/scheduler/storage/db/DbAttributeStore.java > 34f12a9ff0f265561e349800f972a10db41a9fec > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java > 5375ed2cf45b7603d12508e7118a5bded1d87bc4 > src/main/java/org/apache/aurora/scheduler/updater/SideEffect.java > d9e59b6c1270f70235c4d0c38a8df7ce70499768 > src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java > 061d7e7d7d2ac9c7cc585b3c935cac2c572d9226 > > Diff: https://reviews.apache.org/r/25791/diff/ > > > Testing > --- > > ./gradlew build -Pq > > > Thanks, > > Bill Farner > >
Re: Review Request 25721: Asynchronous JS for Scheduler UI
--- 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
Review Request 25794: Add a mock scheduler client to the AuroraClient service
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25794/ --- Review request for Aurora, Joshua Cohen, Kevin Sweeney, Maxim Khutornenko, and Bill Farner. Repository: aurora Description --- Add a mock scheduler client to the AuroraClient service. There is probably a more Angular way to get this done which involves creating a mock service, but this seemed like the simplest approach. Interested in whether this is the right place to do mocking (or should it come from the server so that we have more coverage of the client code?) or if we should mock at all. Code itself needs cleaned up - I just want to solicit feedback before I go too far down a bad path. Diffs - src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 8a719f85b0a095a93b723c04b0a5e8306093c572 Diff: https://reviews.apache.org/r/25794/diff/ Testing --- In Chrome -> Developer Console: window.__mockAPI__ = true; Now I have updates in the UI. Thanks, David McLaughlin
Re: Review Request 25794: Add a mock scheduler client to the AuroraClient service
> On Sept. 18, 2014, 9:53 p.m., Joshua Cohen wrote: > > I don't love the idea of shipping a mock alongside production code. > > > > Is there any way we can separate this out so that it's not included for > > prod (i.e. pull it out of services.js and into a file that's only included > > when running in dev mode)? Alternately, how would you feel about setting up > > a simple mock server that can serve up canned responses (would be easy to > > swap out responses by replacing the mock data on disk...). We could definitely have a separate external test server and then wire in generic code to control the HTTP target (would also let us test against remote schedulers from a local client), it's just a question of where the code for that server and mocks lives. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25794/#review53879 ------- On Sept. 18, 2014, 9:38 p.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25794/ > --- > > (Updated Sept. 18, 2014, 9:38 p.m.) > > > Review request for Aurora, Joshua Cohen, Kevin Sweeney, Maxim Khutornenko, > and Bill Farner. > > > Repository: aurora > > > Description > --- > > Add a mock scheduler client to the AuroraClient service. > > There is probably a more Angular way to get this done which involves creating > a mock service, but this seemed like the simplest approach. > > Interested in whether this is the right place to do mocking (or should it > come from the server so that we have more coverage of the client code?) or if > we should mock at all. > > Code itself needs cleaned up - I just want to solicit feedback before I go > too far down a bad path. > > > Diffs > - > > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js > 8a719f85b0a095a93b723c04b0a5e8306093c572 > > Diff: https://reviews.apache.org/r/25794/diff/ > > > Testing > --- > > In Chrome -> Developer Console: > > window.__mockAPI__ = true; > > Now I have updates in the UI. > > > Thanks, > > David McLaughlin > >
Review Request 25807: Fix the default values for min/max time running while updating
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25807/ --- Review request for Aurora, Maxim Khutornenko and Bill Farner. Repository: aurora Description --- Fix the default values for min/max time running while updating Diffs - src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 13599c021018c5c17a616cc81736c4a42393ce2b src/main/python/apache/aurora/client/api/updater_util.py 178c2fbf0ec02a2c00d5a744fd3ddb5132dafeef Diff: https://reviews.apache.org/r/25807/diff/ Testing --- Thanks, David McLaughlin
Re: Review Request 25835: Serve HTTP assets out of a standard classpath root.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25835/#review54002 --- Reviewboard keeps crashing when I try to go back. But I was a little confused as to why we still need a mapping for some of the static assets? Also, for the goal of faster development iteration on the UI - it would be good to get some docs on expected steps when hacking on the UI in the vagrant image. - David McLaughlin On Sept. 19, 2014, 6:41 p.m., Joshua Cohen wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25835/ > --- > > (Updated Sept. 19, 2014, 6:41 p.m.) > > > Review request for Aurora, David McLaughlin, Kevin Sweeney, and Bill Farner. > > > Bugs: AURORA-678 > https://issues.apache.org/jira/browse/AURORA-678 > > > Repository: aurora > > > Description > --- > > Serve HTTP assets out of a standard claspath root. > > There's a lot of moved files in this diff, so apologies for it being > essentially unreadable. ReviewBoard actually seems to choke about half the > time just loading the individual pages. > > For the sake of convenience/sanity, the actual meat of the changes can be > found in: > > build.gradle: https://reviews.apache.org/r/25835/diff/#336 > src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java: > https://reviews.apache.org/r/25835/diff/1/?page=17#338 > src/main/resources/scheduler/assets/index.html: > https://reviews.apache.org/r/25835/diff/1/?page=18#363 > > Everything else is just a rename/move. > > > Diffs > - > > 3rdparty/javascript/bower_components/angular-bootstrap/.bower.json > 3rdparty/javascript/bower_components/angular-bootstrap/bower.json > 3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap-tpls.js > > > 3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap-tpls.min.js > > 3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap.js > 3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap.min.js > 3rdparty/javascript/bower_components/angular-route/.bower.json > 3rdparty/javascript/bower_components/angular-route/README.md > 3rdparty/javascript/bower_components/angular-route/angular-route.js > 3rdparty/javascript/bower_components/angular-route/angular-route.min.js > 3rdparty/javascript/bower_components/angular-route/angular-route.min.js.map > > 3rdparty/javascript/bower_components/angular-route/bower.json > 3rdparty/javascript/bower_components/angular/.bower.json > 3rdparty/javascript/bower_components/angular/README.md > 3rdparty/javascript/bower_components/angular/angular-csp.css > 3rdparty/javascript/bower_components/angular/angular.js > 3rdparty/javascript/bower_components/angular/angular.min.js > 3rdparty/javascript/bower_components/angular/angular.min.js.gzip > 3rdparty/javascript/bower_components/angular/angular.min.js.map > 3rdparty/javascript/bower_components/angular/bower.json > 3rdparty/javascript/bower_components/bootstrap/.bower.json > 3rdparty/javascript/bower_components/bootstrap/Gruntfile.js > 3rdparty/javascript/bower_components/bootstrap/LICENSE > 3rdparty/javascript/bower_components/bootstrap/README.md > 3rdparty/javascript/bower_components/bootstrap/bower.json > 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.css > > > 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.css.map > > > 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.min.css > > 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.css > 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.css.map > 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.min.css > > 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.eot > > > 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.svg > > > 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.ttf > > > 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.woff > > 3rdparty/javascript/bower_components/bootstrap/dist/js/bootstrap.js > 3rdparty/javascript/bower_components/bootstrap/dist/js/bootstrap.min.js > > 3rdparty/javascript/bower_components/b
Re: Review Request 25835: Serve HTTP assets out of a standard classpath root.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25835/#review54022 --- Ship it! Ship It! - David McLaughlin On Sept. 19, 2014, 8:45 p.m., Joshua Cohen wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25835/ > --- > > (Updated Sept. 19, 2014, 8:45 p.m.) > > > Review request for Aurora, David McLaughlin, Kevin Sweeney, and Bill Farner. > > > Bugs: AURORA-678 > https://issues.apache.org/jira/browse/AURORA-678 > > > Repository: aurora > > > Description > --- > > Serve HTTP assets out of a standard claspath root. > > There's a lot of moved files in this diff, so apologies for it being > essentially unreadable. ReviewBoard actually seems to choke about half the > time just loading the individual pages. > > For the sake of convenience/sanity, the actual meat of the changes can be > found in: > > build.gradle: https://reviews.apache.org/r/25835/diff/#336 > src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java: > https://reviews.apache.org/r/25835/diff/1/?page=17#338 > src/main/resources/scheduler/assets/index.html: > https://reviews.apache.org/r/25835/diff/1/?page=18#363 > > Everything else is just a rename/move. > > > Diffs > - > > 3rdparty/javascript/bower_components/angular-bootstrap/.bower.json > 3rdparty/javascript/bower_components/angular-bootstrap/bower.json > 3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap-tpls.js > > > 3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap-tpls.min.js > > 3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap.js > 3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap.min.js > 3rdparty/javascript/bower_components/angular-route/.bower.json > 3rdparty/javascript/bower_components/angular-route/README.md > 3rdparty/javascript/bower_components/angular-route/angular-route.js > 3rdparty/javascript/bower_components/angular-route/angular-route.min.js > 3rdparty/javascript/bower_components/angular-route/angular-route.min.js.map > > 3rdparty/javascript/bower_components/angular-route/bower.json > 3rdparty/javascript/bower_components/angular/.bower.json > 3rdparty/javascript/bower_components/angular/README.md > 3rdparty/javascript/bower_components/angular/angular-csp.css > 3rdparty/javascript/bower_components/angular/angular.js > 3rdparty/javascript/bower_components/angular/angular.min.js > 3rdparty/javascript/bower_components/angular/angular.min.js.gzip > 3rdparty/javascript/bower_components/angular/angular.min.js.map > 3rdparty/javascript/bower_components/angular/bower.json > 3rdparty/javascript/bower_components/bootstrap/.bower.json > 3rdparty/javascript/bower_components/bootstrap/Gruntfile.js > 3rdparty/javascript/bower_components/bootstrap/LICENSE > 3rdparty/javascript/bower_components/bootstrap/README.md > 3rdparty/javascript/bower_components/bootstrap/bower.json > 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.css > > > 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.css.map > > > 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.min.css > > 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.css > 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.css.map > 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.min.css > > 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.eot > > > 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.svg > > > 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.ttf > > > 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.woff > > 3rdparty/javascript/bower_components/bootstrap/dist/js/bootstrap.js > 3rdparty/javascript/bower_components/bootstrap/dist/js/bootstrap.min.js > > 3rdparty/javascript/bower_components/bootstrap/fonts/glyphicons-halflings-regular.eot > > > 3rdparty/javascript/bower_components/bootstrap/fonts/glyphicons-halflings-regular.svg > > > 3rdparty/javascript/bower_components/bootstrap/fonts/glyphicons-halflings-regular.ttf > > > 3rdpar
Review Request 25859: Fix some bugs in the Update UI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25859/ --- Review request for Aurora, Maxim Khutornenko and Bill Farner. Repository: aurora Description --- Angular has a thing where if you have asynchronous data *and* you bind it to isolated scope in a directive, you need to explicitly watch the values for changes. This was also the weird behaviour I saw before with SmartTable. I changed the ordering of update summaries to be reverse sorted by last event time. Going to be required for a future (show me all in-flight updates) feature. I also fixed a couple of problems with: progress stats, a missing terminal state and the instance summary not updating when polling. Diffs - src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js bcb094adbafdf223eda49c012005994937d5dcf6 src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js 8b7fc0627597d53a788868d0ae16c0c063f36338 src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 8a719f85b0a095a93b723c04b0a5e8306093c572 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml ad877ed168f80cc56561dffd028cff300336ea55 Diff: https://reviews.apache.org/r/25859/diff/ Testing --- ./gradlew jsHint Thanks, David McLaughlin
Re: Review Request 25859: Fix some bugs in the Update UI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25859/ --- (Updated Sept. 22, 2014, 6:24 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Changes --- Fix tests. Repository: aurora Description --- Angular has a thing where if you have asynchronous data *and* you bind it to isolated scope in a directive, you need to explicitly watch the values for changes. This was also the weird behaviour I saw before with SmartTable. I changed the ordering of update summaries to be reverse sorted by last event time. Going to be required for a future (show me all in-flight updates) feature. I also fixed a couple of problems with: progress stats, a missing terminal state and the instance summary not updating when polling. Diffs (updated) - src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js bcb094adbafdf223eda49c012005994937d5dcf6 src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js 8b7fc0627597d53a788868d0ae16c0c063f36338 src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 8a719f85b0a095a93b723c04b0a5e8306093c572 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml ad877ed168f80cc56561dffd028cff300336ea55 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 1cbb7ab9ce4333a1f2f76a7de1940dedb8dc8ecd Diff: https://reviews.apache.org/r/25859/diff/ Testing --- ./gradlew jsHint Thanks, David McLaughlin
Re: Review Request 25859: Fix some bugs in the Update UI
> On Sept. 20, 2014, 1:02 a.m., Maxim Khutornenko wrote: > > src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml, > > line 275 > > <https://reviews.apache.org/r/25859/diff/1/?file=698458#file698458line275> > > > > This will fail unit tests as is. Thanks. Fixed. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25859/#review54066 --- On Sept. 20, 2014, 12:28 a.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25859/ > --- > > (Updated Sept. 20, 2014, 12:28 a.m.) > > > Review request for Aurora, Maxim Khutornenko and Bill Farner. > > > Repository: aurora > > > Description > --- > > Angular has a thing where if you have asynchronous data *and* you bind it to > isolated scope in a directive, you need to explicitly watch the values for > changes. This was also the weird behaviour I saw before with SmartTable. > > I changed the ordering of update summaries to be reverse sorted by last event > time. Going to be required for a future (show me all in-flight updates) > feature. > > I also fixed a couple of problems with: progress stats, a missing terminal > state and the instance summary not updating when polling. > > > Diffs > - > > src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js > bcb094adbafdf223eda49c012005994937d5dcf6 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js > 8b7fc0627597d53a788868d0ae16c0c063f36338 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js > 8a719f85b0a095a93b723c04b0a5e8306093c572 > > src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml > ad877ed168f80cc56561dffd028cff300336ea55 > > Diff: https://reviews.apache.org/r/25859/diff/ > > > Testing > --- > > ./gradlew jsHint > > > Thanks, > > David McLaughlin > >
Review Request 25913: HTML Grid Fixes in Scheduler UI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25913/ --- Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner. Repository: aurora Description --- HTML Grid Fixes in Scheduler UI. Got rid of custom self-closing (void) tags: https://github.com/angular/angular.js/issues/1953. Got rid of nested container-fluids (caused inconsistent margins on role/env pages). Got rid of redundant nested if/else on errors. Diffs - src/main/resources/org/apache/aurora/scheduler/http/ui/breadcrumb.html 5c4fe96e9c6cb816e497f9dc145f99b6568de887 src/main/resources/org/apache/aurora/scheduler/http/ui/error.html 5b03acaf593ee9d0c32e70f7178fb0e54162c7b8 src/main/resources/org/apache/aurora/scheduler/http/ui/job.html ca1a0f666a1eea7c225f017918f4755c28c57502 src/main/resources/org/apache/aurora/scheduler/http/ui/role.html c3a2fd9f12fe6b99a5253f123002a7a02e51a9ec src/main/resources/org/apache/aurora/scheduler/http/ui/update.html b6cf4f0ed44f33bd5048c5ba9267eabb837dab84 Diff: https://reviews.apache.org/r/25913/diff/ Testing --- Manual testing. These are effectively code cleanup no-ops. The main goal is to avoid people cargo-culting existing bad practices in the name of consistency (which I did with the time-display directives). Thanks, David McLaughlin
Re: Review Request 25913: HTML Grid Fixes in Scheduler UI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25913/ --- (Updated Sept. 22, 2014, 11:23 p.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner. Changes --- Added JIRA. Bugs: AURORA-436 https://issues.apache.org/jira/browse/AURORA-436 Repository: aurora Description --- HTML Grid Fixes in Scheduler UI. Got rid of custom self-closing (void) tags: https://github.com/angular/angular.js/issues/1953. Got rid of nested container-fluids (caused inconsistent margins on role/env pages). Got rid of redundant nested if/else on errors. Diffs - src/main/resources/org/apache/aurora/scheduler/http/ui/breadcrumb.html 5c4fe96e9c6cb816e497f9dc145f99b6568de887 src/main/resources/org/apache/aurora/scheduler/http/ui/error.html 5b03acaf593ee9d0c32e70f7178fb0e54162c7b8 src/main/resources/org/apache/aurora/scheduler/http/ui/job.html ca1a0f666a1eea7c225f017918f4755c28c57502 src/main/resources/org/apache/aurora/scheduler/http/ui/role.html c3a2fd9f12fe6b99a5253f123002a7a02e51a9ec src/main/resources/org/apache/aurora/scheduler/http/ui/update.html b6cf4f0ed44f33bd5048c5ba9267eabb837dab84 Diff: https://reviews.apache.org/r/25913/diff/ Testing --- Manual testing. These are effectively code cleanup no-ops. The main goal is to avoid people cargo-culting existing bad practices in the name of consistency (which I did with the time-display directives). Thanks, David McLaughlin
Re: Review Request 25913: HTML Grid Fixes in Scheduler UI
> On Sept. 23, 2014, 12:08 a.m., Joshua Cohen wrote: > > I'm sure it's fine, but in the future it might be helpful to include > > screenshots comparable to those in the ticket showing the proper alignment? Ack. Will add before/after screenshots for this type of work in the future. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25913/#review54209 --- On Sept. 22, 2014, 11:23 p.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25913/ > --- > > (Updated Sept. 22, 2014, 11:23 p.m.) > > > Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner. > > > Bugs: AURORA-436 > https://issues.apache.org/jira/browse/AURORA-436 > > > Repository: aurora > > > Description > --- > > HTML Grid Fixes in Scheduler UI. > > > Got rid of custom self-closing (void) tags: > https://github.com/angular/angular.js/issues/1953. > Got rid of nested container-fluids (caused inconsistent margins on role/env > pages). > Got rid of redundant nested if/else on errors. > > > Diffs > - > > src/main/resources/org/apache/aurora/scheduler/http/ui/breadcrumb.html > 5c4fe96e9c6cb816e497f9dc145f99b6568de887 > src/main/resources/org/apache/aurora/scheduler/http/ui/error.html > 5b03acaf593ee9d0c32e70f7178fb0e54162c7b8 > src/main/resources/org/apache/aurora/scheduler/http/ui/job.html > ca1a0f666a1eea7c225f017918f4755c28c57502 > src/main/resources/org/apache/aurora/scheduler/http/ui/role.html > c3a2fd9f12fe6b99a5253f123002a7a02e51a9ec > src/main/resources/org/apache/aurora/scheduler/http/ui/update.html > b6cf4f0ed44f33bd5048c5ba9267eabb837dab84 > > Diff: https://reviews.apache.org/r/25913/diff/ > > > Testing > --- > > Manual testing. These are effectively code cleanup no-ops. The main goal is > to avoid people cargo-culting existing bad practices in the name of > consistency (which I did with the time-display directives). > > > Thanks, > > David McLaughlin > >
Re: Review Request 25918: Improve aurora command-line help using metavars.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25918/#review54233 --- Ship it! Ship It! - David McLaughlin On Sept. 23, 2014, 1:53 a.m., Mark Chu-Carroll wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25918/ > --- > > (Updated Sept. 23, 2014, 1:53 a.m.) > > > Review request for Aurora, David McLaughlin and Zameer Manji. > > > Bugs: aurora-737 > https://issues.apache.org/jira/browse/aurora-737 > > > Repository: aurora > > > Description > --- > > Improve aurora command-line help using metavars. > > > Diffs > - > > src/main/python/apache/aurora/client/cli/jobs.py > 383c59143ae87d1f059644fdc3433e4c2075d02e > src/main/python/apache/aurora/client/cli/options.py > 36f80aa175beaf8f2dc6b6f962394643313d6405 > src/main/python/apache/aurora/client/cli/task.py > 5b5710ae9005664e17be1053c63b282364ae0300 > > Diff: https://reviews.apache.org/r/25918/diff/ > > > Testing > --- > > All unit tests pass. > > > Thanks, > > Mark Chu-Carroll > >
Re: Review Request 25955: Remove about plugin.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25955/#review54337 --- Ship it! Ship It! - David McLaughlin On Sept. 23, 2014, 9:35 p.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25955/ > --- > > (Updated Sept. 23, 2014, 9:35 p.m.) > > > Review request for Aurora and David McLaughlin. > > > Repository: aurora > > > Description > --- > > I've spent way too much time waiting for this plugin to complete, and it has > a tendency to fail sporadically (especially when running a distribution not > in a git repo). > > Since we don't really use the output anyway, i'm fine to drop it. > > > Diffs > - > > build.gradle e51912116c2a205bb85f229b167ddb3e1b3b7883 > > Diff: https://reviews.apache.org/r/25955/diff/ > > > Testing > --- > > ./gradlew build -Pq > > > Thanks, > > Bill Farner > >
Review Request 25958: Enable beta updater in example cluster.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25958/ --- Review request for Aurora, Maxim Khutornenko and Bill Farner. Repository: aurora Description --- enable beta updater in example cluster Diffs - examples/vagrant/upstart/aurora-scheduler.conf 7e2b3f0191cbc9f0b2ce916a1ad0b55281d718d6 Diff: https://reviews.apache.org/r/25958/diff/ Testing --- $ vagrant provision $ vagrant ssh $ aurora2 update start devcluster/www-data/devel/hello_world hello_world.aurora log(info): Starting update for: hello_world log(info): Starting new HTTP connection (1): 192.168.33.7 log(info): Starting new HTTP connection (1): 192.168.33.7 Scheduler-driven update of job devcluster/www-data/devel/hello_world has started. Thanks, David McLaughlin
Review Request 25963: Expose in progress and recently completed updates in UI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25963/ --- Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner. Bugs: AURORA-745 https://issues.apache.org/jira/browse/AURORA-745 Repository: aurora Description --- Very simple first iteration. Shows only the latest 20 updates. Diffs - src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 392b4f7bc9d5352ed536ff83eafb4175e4860aa1 src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css befd5908e51dcac2a0fc56b12651af11273d25c0 src/main/resources/org/apache/aurora/scheduler/http/ui/index.html 6434d1bc3b9f769f6f565d601fd98f99743efe8c src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js 082d920348f12b214b8ca2d20c11e7b4825453ed src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js ded175a1e4b288b8978235c986db2549c22dcc7d src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js f65cb5e69c2d1712b5f9da782ee2c7b3bc22cee8 src/main/resources/org/apache/aurora/scheduler/http/ui/updateList.html PRE-CREATION Diff: https://reviews.apache.org/r/25963/diff/ Testing --- ./gradlew jsHint File Attachments updates page https://reviews.apache.org/media/uploaded/files/2014/09/23/33b2b9ff-3485-4bba-a8e8-2d4e6280e5ca__Screen_Shot_2014-09-23_at_4.15.06_PM.png no data updates https://reviews.apache.org/media/uploaded/files/2014/09/23/5fe9c0bf-ef31-41a3-95c4-7aeb9abcb3b0__Screen_Shot_2014-09-23_at_4.45.59_PM.png Thanks, David McLaughlin
Re: Review Request 25963: Expose in progress and recently completed updates in UI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25963/#review54363 --- I just realised, we might want to hide the nav if beta_updates_enabled is false. - David McLaughlin On Sept. 23, 2014, 11:46 p.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25963/ > --- > > (Updated Sept. 23, 2014, 11:46 p.m.) > > > Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner. > > > Bugs: AURORA-745 > https://issues.apache.org/jira/browse/AURORA-745 > > > Repository: aurora > > > Description > --- > > Very simple first iteration. Shows only the latest 20 updates. > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java > 392b4f7bc9d5352ed536ff83eafb4175e4860aa1 > src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css > befd5908e51dcac2a0fc56b12651af11273d25c0 > src/main/resources/org/apache/aurora/scheduler/http/ui/index.html > 6434d1bc3b9f769f6f565d601fd98f99743efe8c > src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js > 082d920348f12b214b8ca2d20c11e7b4825453ed > src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js > ded175a1e4b288b8978235c986db2549c22dcc7d > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js > f65cb5e69c2d1712b5f9da782ee2c7b3bc22cee8 > src/main/resources/org/apache/aurora/scheduler/http/ui/updateList.html > PRE-CREATION > > Diff: https://reviews.apache.org/r/25963/diff/ > > > Testing > --- > > ./gradlew jsHint > > > File Attachments > > > updates page > > https://reviews.apache.org/media/uploaded/files/2014/09/23/33b2b9ff-3485-4bba-a8e8-2d4e6280e5ca__Screen_Shot_2014-09-23_at_4.15.06_PM.png > no data updates > > https://reviews.apache.org/media/uploaded/files/2014/09/23/5fe9c0bf-ef31-41a3-95c4-7aeb9abcb3b0__Screen_Shot_2014-09-23_at_4.45.59_PM.png > > > Thanks, > > David McLaughlin > >
Re: Review Request 25963: Expose in progress and recently completed updates in UI
> On Sept. 24, 2014, 4:43 p.m., Joshua Cohen wrote:
> > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js,
> > lines 296-305
> > <https://reviews.apache.org/r/25963/diff/1/?file=703457#file703457line296>
> >
> > Can we just have a single function that takes the statuses as a
> > parameter? If you want to maintain the simplicity of the current API we
> > could bind a more generic function to a specific one. I.e.:
> >
> > function getJobUpdateQuery(status) {
> > var query = new JobUpdateQuery();
> > query.updateStatuses = status;
> > return query;
> > };
> >
> > ...
> >
> > getTerminalQuery: getJobUpdateQuery.bind(this, UPDATE_TERMINAL_STATUSES
> > );
The API is so that services.js was solely responsible for what constitutes a
TERMINAL_STATE, rather than leaking that code into the controllers. There are
multiple ways to do that, of course. For now I will make the refactor you
proposed.
> On Sept. 24, 2014, 4:43 p.m., Joshua Cohen wrote:
> > src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js,
> > line 39
> > <https://reviews.apache.org/r/25963/diff/1/?file=703456#file703456line39>
> >
> > restore the `_MS` suffix to make it explicit what the unit is here?
Fixed.
> On Sept. 24, 2014, 4:43 p.m., Joshua Cohen wrote:
> > src/main/resources/org/apache/aurora/scheduler/http/ui/updateList.html,
> > lines 33-51
> > <https://reviews.apache.org/r/25963/diff/1/?file=703458#file703458line33>
> >
> > This table and the one below for completed updates are identical (minus
> > the data source). Does angular have partials or something similar so we can
> > avoid replicating this?
Sure, I can make this a directive. I figured we might eventually have
pagination/filtering/etc. in the second table but not the first, so that was
the reasoning.. but we can cross that bridge when we come to it.
- David
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25963/#review54422
---
On Sept. 23, 2014, 11:46 p.m., David McLaughlin wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25963/
> ---
>
> (Updated Sept. 23, 2014, 11:46 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner.
>
>
> Bugs: AURORA-745
> https://issues.apache.org/jira/browse/AURORA-745
>
>
> Repository: aurora
>
>
> Description
> ---
>
> Very simple first iteration. Shows only the latest 20 updates.
>
>
> Diffs
> -
>
> src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java
> 392b4f7bc9d5352ed536ff83eafb4175e4860aa1
> src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css
> befd5908e51dcac2a0fc56b12651af11273d25c0
> src/main/resources/org/apache/aurora/scheduler/http/ui/index.html
> 6434d1bc3b9f769f6f565d601fd98f99743efe8c
> src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js
> 082d920348f12b214b8ca2d20c11e7b4825453ed
> src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js
> ded175a1e4b288b8978235c986db2549c22dcc7d
> src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js
> f65cb5e69c2d1712b5f9da782ee2c7b3bc22cee8
> src/main/resources/org/apache/aurora/scheduler/http/ui/updateList.html
> PRE-CREATION
>
> Diff: https://reviews.apache.org/r/25963/diff/
>
>
> Testing
> ---
>
> ./gradlew jsHint
>
>
> File Attachments
> ----
>
> updates page
>
> https://reviews.apache.org/media/uploaded/files/2014/09/23/33b2b9ff-3485-4bba-a8e8-2d4e6280e5ca__Screen_Shot_2014-09-23_at_4.15.06_PM.png
> no data updates
>
> https://reviews.apache.org/media/uploaded/files/2014/09/23/5fe9c0bf-ef31-41a3-95c4-7aeb9abcb3b0__Screen_Shot_2014-09-23_at_4.45.59_PM.png
>
>
> Thanks,
>
> David McLaughlin
>
>
Re: Review Request 25963: Expose in progress and recently completed updates in UI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25963/ --- (Updated Sept. 24, 2014, 9:46 p.m.) Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner. Changes --- feedback. Bugs: AURORA-745 https://issues.apache.org/jira/browse/AURORA-745 Repository: aurora Description --- Very simple first iteration. Shows only the latest 20 updates. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 392b4f7bc9d5352ed536ff83eafb4175e4860aa1 src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css befd5908e51dcac2a0fc56b12651af11273d25c0 src/main/resources/org/apache/aurora/scheduler/http/ui/index.html 6434d1bc3b9f769f6f565d601fd98f99743efe8c src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js 082d920348f12b214b8ca2d20c11e7b4825453ed src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js ded175a1e4b288b8978235c986db2549c22dcc7d src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js 4e67c4e0458cd67df7c1e523ee896b7d2e234a52 src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js f65cb5e69c2d1712b5f9da782ee2c7b3bc22cee8 src/main/resources/org/apache/aurora/scheduler/http/ui/latestUpdates.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/updateList.html PRE-CREATION Diff: https://reviews.apache.org/r/25963/diff/ Testing --- ./gradlew jsHint File Attachments updates page https://reviews.apache.org/media/uploaded/files/2014/09/23/33b2b9ff-3485-4bba-a8e8-2d4e6280e5ca__Screen_Shot_2014-09-23_at_4.15.06_PM.png no data updates https://reviews.apache.org/media/uploaded/files/2014/09/23/5fe9c0bf-ef31-41a3-95c4-7aeb9abcb3b0__Screen_Shot_2014-09-23_at_4.45.59_PM.png Thanks, David McLaughlin
Re: Review Request 26004: Add "aurora update list" and "aurora update status" commands.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26004/#review54466 --- What was the rationale for hiding update IDs from the user and making job key the parameter for update status? It's nice you can quickly see if a job key has an update in progress.. but what happens if you're wanting to see a recent deploy that is already finished or see further back? I think update list should return the update ids and update status should accept an update id. Well maybe not update status.. but update show or something? Something which is consistent with the job verbs would be great. - David McLaughlin On Sept. 24, 2014, 8:24 p.m., Mark Chu-Carroll wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26004/ > --- > > (Updated Sept. 24, 2014, 8:24 p.m.) > > > Review request for Aurora, David McLaughlin and Zameer Manji. > > > Bugs: aurora-742 > https://issues.apache.org/jira/browse/aurora-742 > > > Repository: aurora > > > Description > --- > > Add support for commands to query and display active updates being > managed by the scheduler. Two commands are added: > > * "aurora update list", which shows all active updates that are being > processed by the server. > * "aurora update status", which shows detailed status information about an > update in-progress on the server. > > > Diffs > - > > src/main/python/apache/aurora/client/cli/context.py > 102d20797816788361dfdac450aac9fb8e6fbc28 > src/main/python/apache/aurora/client/cli/options.py > c2d422ac2bc82fc387596e93040b49f722f8310f > src/main/python/apache/aurora/client/cli/update.py > c6cb98fe6aa42310090167796c971856d3dc177f > src/test/python/apache/aurora/client/cli/test_supdate.py > 4fb1623e497b9741ae2a350deb20030dd4036506 > src/test/python/apache/aurora/client/cli/util.py > a50b83c571390374975accf75e31f392dbdaaa04 > > Diff: https://reviews.apache.org/r/26004/diff/ > > > Testing > --- > > Added new unit tests; all tests pass. > > > Thanks, > > Mark Chu-Carroll > >
Re: Review Request 25963: Expose in progress and recently completed updates in UI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25963/ --- (Updated Sept. 24, 2014, 10:52 p.m.) Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner. Bugs: AURORA-745 https://issues.apache.org/jira/browse/AURORA-745 Repository: aurora Description --- Very simple first iteration. Shows only the latest 20 updates. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 392b4f7bc9d5352ed536ff83eafb4175e4860aa1 src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css befd5908e51dcac2a0fc56b12651af11273d25c0 src/main/resources/org/apache/aurora/scheduler/http/ui/index.html 6434d1bc3b9f769f6f565d601fd98f99743efe8c src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js 082d920348f12b214b8ca2d20c11e7b4825453ed src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js ded175a1e4b288b8978235c986db2549c22dcc7d src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js 4e67c4e0458cd67df7c1e523ee896b7d2e234a52 src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js f65cb5e69c2d1712b5f9da782ee2c7b3bc22cee8 src/main/resources/org/apache/aurora/scheduler/http/ui/latestUpdates.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/updateList.html PRE-CREATION Diff: https://reviews.apache.org/r/25963/diff/ Testing --- ./gradlew jsHint File Attachments updates page https://reviews.apache.org/media/uploaded/files/2014/09/23/33b2b9ff-3485-4bba-a8e8-2d4e6280e5ca__Screen_Shot_2014-09-23_at_4.15.06_PM.png no data updates https://reviews.apache.org/media/uploaded/files/2014/09/23/5fe9c0bf-ef31-41a3-95c4-7aeb9abcb3b0__Screen_Shot_2014-09-23_at_4.45.59_PM.png Thanks, David McLaughlin
Re: Review Request 25963: Expose in progress and recently completed updates in UI
> On Sept. 24, 2014, 10:23 p.m., Joshua Cohen wrote: > > src/main/resources/org/apache/aurora/scheduler/http/ui/latestUpdates.html, > > line 15 > > <https://reviews.apache.org/r/25963/diff/2/?file=704724#file704724line15> > > > > Nit: should we parameterize this to maintain the previous distinction > > between in progress vs completed? Fixed. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25963/#review54465 ------- On Sept. 24, 2014, 9:46 p.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25963/ > --- > > (Updated Sept. 24, 2014, 9:46 p.m.) > > > Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner. > > > Bugs: AURORA-745 > https://issues.apache.org/jira/browse/AURORA-745 > > > Repository: aurora > > > Description > --- > > Very simple first iteration. Shows only the latest 20 updates. > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java > 392b4f7bc9d5352ed536ff83eafb4175e4860aa1 > src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css > befd5908e51dcac2a0fc56b12651af11273d25c0 > src/main/resources/org/apache/aurora/scheduler/http/ui/index.html > 6434d1bc3b9f769f6f565d601fd98f99743efe8c > src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js > 082d920348f12b214b8ca2d20c11e7b4825453ed > src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js > ded175a1e4b288b8978235c986db2549c22dcc7d > src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js > 4e67c4e0458cd67df7c1e523ee896b7d2e234a52 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js > f65cb5e69c2d1712b5f9da782ee2c7b3bc22cee8 > src/main/resources/org/apache/aurora/scheduler/http/ui/latestUpdates.html > PRE-CREATION > src/main/resources/org/apache/aurora/scheduler/http/ui/updateList.html > PRE-CREATION > > Diff: https://reviews.apache.org/r/25963/diff/ > > > Testing > --- > > ./gradlew jsHint > > > File Attachments > > > updates page > > https://reviews.apache.org/media/uploaded/files/2014/09/23/33b2b9ff-3485-4bba-a8e8-2d4e6280e5ca__Screen_Shot_2014-09-23_at_4.15.06_PM.png > no data updates > > https://reviews.apache.org/media/uploaded/files/2014/09/23/5fe9c0bf-ef31-41a3-95c4-7aeb9abcb3b0__Screen_Shot_2014-09-23_at_4.45.59_PM.png > > > Thanks, > > David McLaughlin > >
Re: Review Request 26012: Use BufferedOutputStream for deflater
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26012/#review54494 --- Ship it! Ship It! - David McLaughlin On Sept. 25, 2014, 1:36 a.m., Kevin Sweeney wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26012/ > --- > > (Updated Sept. 25, 2014, 1:36 a.m.) > > > Review request for Aurora, David McLaughlin, Joe Smith, and Bill Farner. > > > Repository: aurora > > > Description > --- > > Use BufferedOutputStream for deflater > > > Diffs > - > > src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java > 45cf7ecf52582ba3fd7bbc8b4a981e396793da4b > > Diff: https://reviews.apache.org/r/26012/diff/ > > > Testing > --- > > ./gradlew build > > Also put together a microbenchmark with a scheduler backup from my production > environment - using a BufferedOutputStream gets a 4sec speedup with no > reduction in compression ratio. ¯_(?)_/¯ > > # Deflater.BEST_SPEED, No buffer > deflate: 11745ms > total:12734ms > compression ratio: 647270930/121616834 = 532.22% > > # Deflater.BEST_SPEED, buf = 512KiB > deflate: 11015ms > total:12001ms > compression ratio: 647270930/121616834 = 532.22% > > # Deflater.BEST_SPEED, BufferedOutputStream > deflate: 6885ms > total:7694ms > compression ratio: 647270930/121616834 = 532.22% > > # Deflater.BEST_SPEED, BufferedOutputStream at 512KiB > deflate: 6752ms > total:7585ms > compression ratio: 647270930/121616834 = 532.22% > > # Deflater.DEFAULT_COMPRESSION > deflate: 16664ms > total:17219ms > compression ratio: 647270930/92690409 = 698.31% > > # Deflater.DEFAULT_COMPRESSION, buf size = 512KiB > deflate: 16516ms > total:17103ms > compression ratio: 647270930/92690409 = 698.31% > > # Deflater.DEFAULT_COMPRESSION, BufferedOutputStream > deflate: 12241ms > total:12788ms > compression ratio: 647270930/92690409 = 698.31% > > # Deflater.DEFAULT_COMPRESSION, BufferedOutputStream at 512KiB > deflate: 11548ms > total:12108ms > compression ratio: 647270930/92690409 = 698.31% > > > Thanks, > > Kevin Sweeney > >
Review Request 26048: Cosmetic UI changes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26048/ --- Review request for Aurora and Bill Farner. Repository: aurora Description --- Move the breadcumb to be under the nav (like on our public site) and make the page title have consistent markup across pages. Diffs - src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css 509639e122821057263703e7662c3e7d659d1c29 src/main/resources/org/apache/aurora/scheduler/http/ui/role.html d305fb58c502dc4dba74fe8557752a5c86da9eb3 Diff: https://reviews.apache.org/r/26048/diff/ Testing --- File Attachments breadcrumb-after https://reviews.apache.org/media/uploaded/files/2014/09/25/0ca9cd6e-a742-40ca-b054-bd409bf66d2a__ui-breadcrumb.png env page after https://reviews.apache.org/media/uploaded/files/2014/09/25/93794e00-6db7-48cd-85e6-6c1550e0957e__ui-env.png role page after https://reviews.apache.org/media/uploaded/files/2014/09/25/2e4590ba-dec9-46a8-8d6e-dde0319e545e__ui-role.png breadcrumb before https://reviews.apache.org/media/uploaded/files/2014/09/25/e37047de-56ee-4f48-91c3-a7f8741571de__ui-breadcrumb-before.png role page before https://reviews.apache.org/media/uploaded/files/2014/09/25/f1fcc383-9757-4632-9dc6-334d7bfb457e__ui-role-before.png env page before https://reviews.apache.org/media/uploaded/files/2014/09/25/a948b4cb-94d4-4e7b-86a9-ecf5d59ef901__ui-env-before.png Thanks, David McLaughlin
Review Request 26095: Fix instance summary visualisation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26095/ --- Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-755 https://issues.apache.org/jira/browse/AURORA-755 Repository: aurora Description --- See JIRA ticket. Diffs - src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js 92c78c28020d36b978f1a098202174015ace1316 src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 8ed95737ea69112d4ee04b687281d06a21d6abf6 Diff: https://reviews.apache.org/r/26095/diff/ Testing --- File Attachments Growing a job shows correct instance ids https://reviews.apache.org/media/uploaded/files/2014/09/26/246430ff-cf02-4701-9254-6f8e42f95b2a__Screen_Shot_2014-09-26_at_12.47.45_PM.png Instances removed are identified when shrinking job from 5 instances to 1 https://reviews.apache.org/media/uploaded/files/2014/09/26/b01c5d23-080e-4433-804b-c868215c0bcb__Screen_Shot_2014-09-26_at_12.47.56_PM.png Thanks, David McLaughlin
Re: Review Request 26095: Fix instance summary visualisation.
> On Sept. 26, 2014, 8:19 p.m., Joshua Cohen wrote: > > File Attachment: Instances removed are identified when shrinking job from 5 > > instances to 1 - Screen Shot 2014-09-26 at 12.47.56 PM.png > > <https://reviews.apache.org/r/26095/#fcomment29> > > > > Should this be a different color? > > > > (in case that's not showing up properly, "this" is the box for the > > removed instance) Removed instances have a different border color. So it's green (success) + black (the great void). Successful roll back looks the same (red border). > On Sept. 26, 2014, 8:19 p.m., Joshua Cohen wrote: > > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js, line > > 328 > > <https://reviews.apache.org/r/26095/diff/1/?file=706256#file706256line328> > > > > nit, since this function doesn't actually return the instance ids, but > > rather performs a callback on them, perhaps name it something more in line > > with its actual behavior? `actOnInstanceIdsFromRange` or `forEachInRange` Fixed. > On Sept. 26, 2014, 8:19 p.m., Joshua Cohen wrote: > > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js, line > > 439 > > <https://reviews.apache.org/r/26095/diff/1/?file=706256#file706256line439> > > > > Why the extra parens around > > `(updateUtil.getAllInstanceIds(details.update))` I have no idea. Removed. > On Sept. 26, 2014, 8:19 p.m., Joshua Cohen wrote: > > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js, line > > 440 > > <https://reviews.apache.org/r/26095/diff/1/?file=706256#file706256line440> > > > > As far as I can see, allInstances is only used here to get the length, > > can we just make this: > > > > var totalInstancesToBeUpdated = > > Object.keys(updateUtil.getAllInstanceIds(details.update).allIds).length Right. Did it this way because that one-liner exceeds 100 characters. - David ------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26095/#review54716 --- On Sept. 26, 2014, 7:55 p.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26095/ > --- > > (Updated Sept. 26, 2014, 7:55 p.m.) > > > Review request for Aurora, Maxim Khutornenko and Bill Farner. > > > Bugs: AURORA-755 > https://issues.apache.org/jira/browse/AURORA-755 > > > Repository: aurora > > > Description > --- > > See JIRA ticket. > > > Diffs > - > > src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js > 92c78c28020d36b978f1a098202174015ace1316 > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js > 8ed95737ea69112d4ee04b687281d06a21d6abf6 > > Diff: https://reviews.apache.org/r/26095/diff/ > > > Testing > --- > > > File Attachments > > > Growing a job shows correct instance ids > > https://reviews.apache.org/media/uploaded/files/2014/09/26/246430ff-cf02-4701-9254-6f8e42f95b2a__Screen_Shot_2014-09-26_at_12.47.45_PM.png > Instances removed are identified when shrinking job from 5 instances to 1 > > https://reviews.apache.org/media/uploaded/files/2014/09/26/b01c5d23-080e-4433-804b-c868215c0bcb__Screen_Shot_2014-09-26_at_12.47.56_PM.png > > > Thanks, > > David McLaughlin > >
Re: Review Request 26095: Fix instance summary visualisation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26095/ --- (Updated Sept. 29, 2014, 4:11 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Changes --- rb feedback. Bugs: AURORA-755 https://issues.apache.org/jira/browse/AURORA-755 Repository: aurora Description --- See JIRA ticket. Diffs (updated) - src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js 92c78c28020d36b978f1a098202174015ace1316 src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 8ed95737ea69112d4ee04b687281d06a21d6abf6 Diff: https://reviews.apache.org/r/26095/diff/ Testing --- File Attachments Growing a job shows correct instance ids https://reviews.apache.org/media/uploaded/files/2014/09/26/246430ff-cf02-4701-9254-6f8e42f95b2a__Screen_Shot_2014-09-26_at_12.47.45_PM.png Instances removed are identified when shrinking job from 5 instances to 1 https://reviews.apache.org/media/uploaded/files/2014/09/26/b01c5d23-080e-4433-804b-c868215c0bcb__Screen_Shot_2014-09-26_at_12.47.56_PM.png Thanks, David McLaughlin
Re: Review Request 25835: Serve HTTP assets out of a standard classpath root.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25835/#review54854 --- Ship it! Ship It! - David McLaughlin On Sept. 19, 2014, 9:39 p.m., Joshua Cohen wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25835/ > --- > > (Updated Sept. 19, 2014, 9:39 p.m.) > > > Review request for Aurora, David McLaughlin, Kevin Sweeney, and Bill Farner. > > > Bugs: AURORA-678 > https://issues.apache.org/jira/browse/AURORA-678 > > > Repository: aurora > > > Description > --- > > Serve HTTP assets out of a standard claspath root. > > There's a lot of moved files in this diff, so apologies for it being > essentially unreadable. ReviewBoard actually seems to choke about half the > time just loading the individual pages. > > For the sake of convenience/sanity, the actual meat of the changes can be > found in: > > build.gradle: https://reviews.apache.org/r/25835/diff/#336 > src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java: > https://reviews.apache.org/r/25835/diff/1/?page=17#338 > src/main/resources/scheduler/assets/index.html: > https://reviews.apache.org/r/25835/diff/1/?page=18#363 > > Everything else is just a rename/move. > > > Diffs > - > > 3rdparty/javascript/bower_components/angular-bootstrap/.bower.json > 3rdparty/javascript/bower_components/angular-bootstrap/bower.json > 3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap-tpls.js > > > 3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap-tpls.min.js > > 3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap.js > 3rdparty/javascript/bower_components/angular-bootstrap/ui-bootstrap.min.js > 3rdparty/javascript/bower_components/angular-route/.bower.json > 3rdparty/javascript/bower_components/angular-route/README.md > 3rdparty/javascript/bower_components/angular-route/angular-route.js > 3rdparty/javascript/bower_components/angular-route/angular-route.min.js > 3rdparty/javascript/bower_components/angular-route/angular-route.min.js.map > > 3rdparty/javascript/bower_components/angular-route/bower.json > 3rdparty/javascript/bower_components/angular/.bower.json > 3rdparty/javascript/bower_components/angular/README.md > 3rdparty/javascript/bower_components/angular/angular-csp.css > 3rdparty/javascript/bower_components/angular/angular.js > 3rdparty/javascript/bower_components/angular/angular.min.js > 3rdparty/javascript/bower_components/angular/angular.min.js.gzip > 3rdparty/javascript/bower_components/angular/angular.min.js.map > 3rdparty/javascript/bower_components/angular/bower.json > 3rdparty/javascript/bower_components/bootstrap/.bower.json > 3rdparty/javascript/bower_components/bootstrap/Gruntfile.js > 3rdparty/javascript/bower_components/bootstrap/LICENSE > 3rdparty/javascript/bower_components/bootstrap/README.md > 3rdparty/javascript/bower_components/bootstrap/bower.json > 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.css > > > 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.css.map > > > 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap-theme.min.css > > 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.css > 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.css.map > 3rdparty/javascript/bower_components/bootstrap/dist/css/bootstrap.min.css > > 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.eot > > > 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.svg > > > 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.ttf > > > 3rdparty/javascript/bower_components/bootstrap/dist/fonts/glyphicons-halflings-regular.woff > > 3rdparty/javascript/bower_components/bootstrap/dist/js/bootstrap.js > 3rdparty/javascript/bower_components/bootstrap/dist/js/bootstrap.min.js > > 3rdparty/javascript/bower_components/bootstrap/fonts/glyphicons-halflings-regular.eot > > > 3rdparty/javascript/bower_components/bootstrap/fonts/glyphicons-halflings-regular.svg > > > 3rdparty/javascript/bower_components/bootstrap/fonts/glyphicons-halflings-regular.ttf > > > 3rdpar
Review Request 26239: Add usernames to scheduler update operations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26239/ --- Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-772 https://issues.apache.org/jira/browse/AURORA-772 Repository: aurora Description --- Add usernames to scheduler update operations. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 01fc345344e4ae807607f8f87e8a9974c3b69151 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java b8dafe077999c1f2d14bbc260c83386020460396 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java faa21363b87505e4212574bb9872d1e03a0e8f24 src/main/resources/org/apache/aurora/scheduler/http/ui/update.html aaff5b21f3d20f00eaf98c29d0e5bf19b25f2f62 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml 631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateEventMapper.xml 9b7e8ba620b42cfb404c9c1440f953918c73 src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 2894b617af082bfde1d44571868200271b38724d src/main/thrift/org/apache/aurora/gen/api.thrift a1217edbcd36cbe02b09a549b71e87ee40ffc6c7 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 60c1582d4211b79656797a84ca6a7a67c7fecdfe src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 8843990484756664a0c16c61303f1aa992e7686d Diff: https://reviews.apache.org/r/26239/diff/ Testing --- ./gradlew -Pq build Thanks, David McLaughlin
Re: Review Request 26004: Add "aurora update list" and "aurora update status" commands.
> On Sept. 24, 2014, 10:28 p.m., David McLaughlin wrote: > > What was the rationale for hiding update IDs from the user and making job > > key the parameter for update status? It's nice you can quickly see if a job > > key has an update in progress.. but what happens if you're wanting to see a > > recent deploy that is already finished or see further back? > > > > I think update list should return the update ids and update status should > > accept an update id. Well maybe not update status.. but update show or > > something? Something which is consistent with the job verbs would be great. > > Mark Chu-Carroll wrote: > I was following the pattern that I saw in the API implementation. In > early iterations, it exposed updated ids; in later ones, the IDs were removed > from the user-level API. We don't use those identifiers for pausing or > resuming an active update; why would we use them for checking on the status > of an update? That would become a very awkward interface for users: start an > update? use the jobkey. Pause an update? User the jobkey. Abort an update? > Use the jobkey. Check the status of an update? Use some other identifier that > you don't know without running another command. > > I can certainly add the updateID to the list command, and add a command > (or alternative parameter) for checking on a job by update-ID, but I don't > think that making it the normal parameter to status is a good design for the > command-line. > > Mark Chu-Carroll wrote: > ping? Can we add updateID to the list command? Then this looks ready to ship. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26004/#review54466 --- On Sept. 24, 2014, 8:24 p.m., Mark Chu-Carroll wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26004/ > --- > > (Updated Sept. 24, 2014, 8:24 p.m.) > > > Review request for Aurora, David McLaughlin and Zameer Manji. > > > Bugs: aurora-742 > https://issues.apache.org/jira/browse/aurora-742 > > > Repository: aurora > > > Description > --- > > Add support for commands to query and display active updates being > managed by the scheduler. Two commands are added: > > * "aurora update list", which shows all active updates that are being > processed by the server. > * "aurora update status", which shows detailed status information about an > update in-progress on the server. > > > Diffs > - > > src/main/python/apache/aurora/client/cli/context.py > 102d20797816788361dfdac450aac9fb8e6fbc28 > src/main/python/apache/aurora/client/cli/options.py > c2d422ac2bc82fc387596e93040b49f722f8310f > src/main/python/apache/aurora/client/cli/update.py > c6cb98fe6aa42310090167796c971856d3dc177f > src/test/python/apache/aurora/client/cli/test_supdate.py > 4fb1623e497b9741ae2a350deb20030dd4036506 > src/test/python/apache/aurora/client/cli/util.py > a50b83c571390374975accf75e31f392dbdaaa04 > > Diff: https://reviews.apache.org/r/26004/diff/ > > > Testing > --- > > Added new unit tests; all tests pass. > > > Thanks, > > Mark Chu-Carroll > >
Re: Review Request 26232: Implementing update history pruner.
> On Oct. 1, 2014, 5:42 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java, line 161 > > <https://reviews.apache.org/r/26232/diff/1/?file=710137#file710137line161> > > > > We also need time-based retention. How about 1 month by default? > > Maxim Khutornenko wrote: > Thought about that but was not sure the extra complexity is worth it. > Also, don't we want to show update history in the UI regardless of how old it > is? > > Bill Farner wrote: > Given that it currently needs to fit in memory, i think not. > > For example: if i create a job to test something out, then kill it and > never return. Probably shouldn't retain those updates forever. Wouldn't the updates in that case be killed by the job garbage collection/cascading deletion of the job key? - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26232/#review55088 --- On Oct. 1, 2014, 5:26 p.m., Maxim Khutornenko wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26232/ > ------- > > (Updated Oct. 1, 2014, 5:26 p.m.) > > > Review request for Aurora, David McLaughlin and Bill Farner. > > > Bugs: AURORA-743 > https://issues.apache.org/jira/browse/AURORA-743 > > > Repository: aurora > > > Description > --- > > The pruner runs on periodic basis and trims completed updates up to the > guranteed per job retention count. > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java > aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef > src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java > ebae58a04e8857c5f26d4b57c27dfcda9e14c82c > src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java > PRE-CREATION > src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java > c3abffe575e801cebec3572cf4aceac83a238b55 > src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java > 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 > > src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java > c583e085e0458835d51ebf740a3b5f01b428bb25 > > src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java > 66c91644677e7176ccf53dcfcf29a6792ec398bc > > src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml > 631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 > src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java > 011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee > > src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java > PRE-CREATION > > src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java > 1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 > > Diff: https://reviews.apache.org/r/26232/diff/ > > > Testing > --- > > ./gradlew -Pq build > > > Thanks, > > Maxim Khutornenko > >
Re: Review Request 26239: Add usernames to scheduler update operations.
> On Oct. 1, 2014, 6:49 p.m., Bill Farner wrote: > > src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateEventMapper.xml, > > line 33 > > <https://reviews.apache.org/r/26239/diff/1/?file=710195#file710195line33> > > > > How does this present when the value is null? Empty string? Yes. The frustrating thing is we can't assume null = system events, because for backwards compatibility we need to account for existing update events from before this patch having null users too. > On Oct. 1, 2014, 6:49 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java, > > line 52 > > <https://reviews.apache.org/r/26239/diff/1/?file=710191#file710191line52> > > > > please doc new @params on javdocs throughout Fixed. > On Oct. 1, 2014, 6:49 p.m., Bill Farner wrote: > > src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java, > > line 688 > > <https://reviews.apache.org/r/26239/diff/1/?file=710198#file710198line688> > > > > line break above, to visually separate the wrapped signature from > > method body. And of course I based my style on the nearest existing method, which didn't have that. Fixed both of them. > On Oct. 1, 2014, 6:49 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, > > line 288 > > <https://reviews.apache.org/r/26239/diff/1/?file=710192#file710192line288> > > > > Since you allow an absent value, use Optional > > > > Ditt down the call stack. > > Bill Farner wrote: > Ditto* Fixed. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26239/#review55110 --- On Oct. 1, 2014, 6:42 p.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26239/ > --- > > (Updated Oct. 1, 2014, 6:42 p.m.) > > > Review request for Aurora, Maxim Khutornenko and Bill Farner. > > > Bugs: AURORA-772 > https://issues.apache.org/jira/browse/AURORA-772 > > > Repository: aurora > > > Description > --- > > Add usernames to scheduler update operations. > > > Diffs > - > > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java > 01fc345344e4ae807607f8f87e8a9974c3b69151 > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java > b8dafe077999c1f2d14bbc260c83386020460396 > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java > faa21363b87505e4212574bb9872d1e03a0e8f24 > src/main/resources/org/apache/aurora/scheduler/http/ui/update.html > aaff5b21f3d20f00eaf98c29d0e5bf19b25f2f62 > > src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml > 631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 > > src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateEventMapper.xml > 9b7e8ba620b42cfb404c9c1440f953918c73 > src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql > 2894b617af082bfde1d44571868200271b38724d > src/main/thrift/org/apache/aurora/gen/api.thrift > a1217edbcd36cbe02b09a549b71e87ee40ffc6c7 > > src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java > 1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 > > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java > 60c1582d4211b79656797a84ca6a7a67c7fecdfe > src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java > 8843990484756664a0c16c61303f1aa992e7686d > > Diff: https://reviews.apache.org/r/26239/diff/ > > > Testing > --- > > ./gradlew -Pq build > > > Thanks, > > David McLaughlin > >
Re: Review Request 26239: Add usernames to scheduler update operations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26239/ --- (Updated Oct. 1, 2014, 7:35 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-772 https://issues.apache.org/jira/browse/AURORA-772 Repository: aurora Description --- Add usernames to scheduler update operations. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 01fc345344e4ae807607f8f87e8a9974c3b69151 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java b8dafe077999c1f2d14bbc260c83386020460396 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java faa21363b87505e4212574bb9872d1e03a0e8f24 src/main/resources/org/apache/aurora/scheduler/http/ui/update.html aaff5b21f3d20f00eaf98c29d0e5bf19b25f2f62 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml 631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateEventMapper.xml 9b7e8ba620b42cfb404c9c1440f953918c73 src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 2894b617af082bfde1d44571868200271b38724d src/main/thrift/org/apache/aurora/gen/api.thrift a1217edbcd36cbe02b09a549b71e87ee40ffc6c7 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 60c1582d4211b79656797a84ca6a7a67c7fecdfe src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 8843990484756664a0c16c61303f1aa992e7686d Diff: https://reviews.apache.org/r/26239/diff/ Testing --- ./gradlew -Pq build Thanks, David McLaughlin
Re: Review Request 26137: Fix help for new update command.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26137/#review55125 --- Ship it! Ship It! - David McLaughlin On Sept. 29, 2014, 3:05 p.m., Mark Chu-Carroll wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26137/ > --- > > (Updated Sept. 29, 2014, 3:05 p.m.) > > > Review request for Aurora, David McLaughlin and Zameer Manji. > > > Bugs: aurora-748 > https://issues.apache.org/jira/browse/aurora-748 > > > Repository: aurora > > > Description > --- > > Fix help for new update command. > > > Diffs > - > > src/main/python/apache/aurora/client/cli/update.py > b4dd792dc12f19424c620f4d91748113e272f0c9 > > Diff: https://reviews.apache.org/r/26137/diff/ > > > Testing > --- > > manual testing. > > > Thanks, > > Mark Chu-Carroll > >
Re: Review Request 26137: Fix help for new update command.
> On Sept. 30, 2014, 6:32 a.m., Joe Smith wrote: > > src/main/python/apache/aurora/client/cli/update.py, line 45 > > <https://reviews.apache.org/r/26137/diff/1/?file=708198#file708198line45> > > > > Could you update a test case to catch accessing these as properties to > > catch accidental regressions? Piggy backing this issue to add that my ship it is pending a test for this command at least? - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26137/#review54953 --- On Sept. 29, 2014, 3:05 p.m., Mark Chu-Carroll wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26137/ > --- > > (Updated Sept. 29, 2014, 3:05 p.m.) > > > Review request for Aurora, David McLaughlin and Zameer Manji. > > > Bugs: aurora-748 > https://issues.apache.org/jira/browse/aurora-748 > > > Repository: aurora > > > Description > --- > > Fix help for new update command. > > > Diffs > - > > src/main/python/apache/aurora/client/cli/update.py > b4dd792dc12f19424c620f4d91748113e272f0c9 > > Diff: https://reviews.apache.org/r/26137/diff/ > > > Testing > --- > > manual testing. > > > Thanks, > > Mark Chu-Carroll > >
Re: Review Request 26239: Add usernames to scheduler update operations.
> On Oct. 2, 2014, 1:29 a.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java, > > line 53 > > <https://reviews.apache.org/r/26239/diff/2/?file=710308#file710308line53> > > > > s/id/name/g Fixed. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26239/#review55175 ------- On Oct. 2, 2014, 2:29 a.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26239/ > --- > > (Updated Oct. 2, 2014, 2:29 a.m.) > > > Review request for Aurora, Maxim Khutornenko and Bill Farner. > > > Bugs: AURORA-772 > https://issues.apache.org/jira/browse/AURORA-772 > > > Repository: aurora > > > Description > --- > > Add usernames to scheduler update operations. > > > Diffs > - > > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java > 01fc345344e4ae807607f8f87e8a9974c3b69151 > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java > b8dafe077999c1f2d14bbc260c83386020460396 > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java > faa21363b87505e4212574bb9872d1e03a0e8f24 > src/main/resources/org/apache/aurora/scheduler/http/ui/update.html > aaff5b21f3d20f00eaf98c29d0e5bf19b25f2f62 > > src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml > 631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 > > src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateEventMapper.xml > 9b7e8ba620b42cfb404c9c1440f953918c73 > src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql > 2894b617af082bfde1d44571868200271b38724d > src/main/thrift/org/apache/aurora/gen/api.thrift > a1217edbcd36cbe02b09a549b71e87ee40ffc6c7 > > src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java > 1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 > > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java > 60c1582d4211b79656797a84ca6a7a67c7fecdfe > src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java > 8843990484756664a0c16c61303f1aa992e7686d > > Diff: https://reviews.apache.org/r/26239/diff/ > > > Testing > --- > > ./gradlew -Pq build > > > Thanks, > > David McLaughlin > >
Re: Review Request 26239: Add usernames to scheduler update operations.
> On Oct. 1, 2014, 10:42 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, > > line 383 > > <https://reviews.apache.org/r/26239/diff/2/?file=710309#file710309line383> > > > > I'd actually opt for orNull() here. An empty string might be confused > > with the auth system spitting out empty. Fixed. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26239/#review55154 ------- On Oct. 2, 2014, 2:29 a.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26239/ > --- > > (Updated Oct. 2, 2014, 2:29 a.m.) > > > Review request for Aurora, Maxim Khutornenko and Bill Farner. > > > Bugs: AURORA-772 > https://issues.apache.org/jira/browse/AURORA-772 > > > Repository: aurora > > > Description > --- > > Add usernames to scheduler update operations. > > > Diffs > - > > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java > 01fc345344e4ae807607f8f87e8a9974c3b69151 > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java > b8dafe077999c1f2d14bbc260c83386020460396 > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java > faa21363b87505e4212574bb9872d1e03a0e8f24 > src/main/resources/org/apache/aurora/scheduler/http/ui/update.html > aaff5b21f3d20f00eaf98c29d0e5bf19b25f2f62 > > src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml > 631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 > > src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateEventMapper.xml > 9b7e8ba620b42cfb404c9c1440f953918c73 > src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql > 2894b617af082bfde1d44571868200271b38724d > src/main/thrift/org/apache/aurora/gen/api.thrift > a1217edbcd36cbe02b09a549b71e87ee40ffc6c7 > > src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java > 1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 > > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java > 60c1582d4211b79656797a84ca6a7a67c7fecdfe > src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java > 8843990484756664a0c16c61303f1aa992e7686d > > Diff: https://reviews.apache.org/r/26239/diff/ > > > Testing > --- > > ./gradlew -Pq build > > > Thanks, > > David McLaughlin > >
Re: Review Request 26239: Add usernames to scheduler update operations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26239/ --- (Updated Oct. 2, 2014, 2:29 a.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Changes --- rb feedback. Bugs: AURORA-772 https://issues.apache.org/jira/browse/AURORA-772 Repository: aurora Description --- Add usernames to scheduler update operations. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 01fc345344e4ae807607f8f87e8a9974c3b69151 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java b8dafe077999c1f2d14bbc260c83386020460396 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java faa21363b87505e4212574bb9872d1e03a0e8f24 src/main/resources/org/apache/aurora/scheduler/http/ui/update.html aaff5b21f3d20f00eaf98c29d0e5bf19b25f2f62 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml 631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateEventMapper.xml 9b7e8ba620b42cfb404c9c1440f953918c73 src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 2894b617af082bfde1d44571868200271b38724d src/main/thrift/org/apache/aurora/gen/api.thrift a1217edbcd36cbe02b09a549b71e87ee40ffc6c7 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 60c1582d4211b79656797a84ca6a7a67c7fecdfe src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 8843990484756664a0c16c61303f1aa992e7686d Diff: https://reviews.apache.org/r/26239/diff/ Testing --- ./gradlew -Pq build Thanks, David McLaughlin
Re: Review Request 26270: Fix multiple errors involving bindings, and fix result codes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26270/#review55239 --- Ship it! Ship It! - David McLaughlin On Oct. 2, 2014, 6:24 p.m., Mark Chu-Carroll wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26270/ > --- > > (Updated Oct. 2, 2014, 6:24 p.m.) > > > Review request for Aurora, David McLaughlin and Zameer Manji. > > > Bugs: aurora-781 > https://issues.apache.org/jira/browse/aurora-781 > > > Repository: aurora > > > Description > --- > > Fix multiple errors involving bindings, and fix result codes. > > - Cause an error to be raised by an unresolved binding in a configuration > file. > - Fix incorrect structuring of processed bindings. > - Add a test to confirm that unbound parameters generate errors. > > While I'm in the code, also fix a problem where result codes aren't returned > correctly > to the shell. > > > Diffs > - > > src/main/python/apache/aurora/client/cli/client.py > 0cb69448cd24372067ac845eca5862bc3d3a46a9 > src/main/python/apache/aurora/client/cli/context.py > 102d20797816788361dfdac450aac9fb8e6fbc28 > src/main/python/apache/aurora/client/cli/options.py > c2d422ac2bc82fc387596e93040b49f722f8310f > src/main/python/apache/aurora/client/cli/standalone_client.py > 30d8f750559b7811d66760741905fa8adf80fd1f > src/test/python/apache/aurora/client/cli/test_create.py > 31fa56f5edcfc97903725ab27ccc12c6a8f39ffc > src/test/python/apache/aurora/client/cli/util.py > a50b83c571390374975accf75e31f392dbdaaa04 > > Diff: https://reviews.apache.org/r/26270/diff/ > > > Testing > --- > > Unit tests: > - verified that tests fail if the binding code is disabled, but pass with in > on. > - added a new unit test to check that unresolved bindings generate an error. > > > Thanks, > > Mark Chu-Carroll > >
Re: Review Request 26232: Implementing update history pruner.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26232/#review55272 --- src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml <https://reviews.apache.org/r/26232/#comment95663> It'd be nice if this didn't just blanket delete all old updates, especially for active jobs. There are probably a certain class of service/cron that are rarely touched and it would be nice to keep around that change history. Would it add too much complexity to try and solve this? - David McLaughlin On Oct. 2, 2014, 10:51 p.m., Maxim Khutornenko wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26232/ > --- > > (Updated Oct. 2, 2014, 10:51 p.m.) > > > Review request for Aurora, David McLaughlin and Bill Farner. > > > Bugs: AURORA-743 > https://issues.apache.org/jira/browse/AURORA-743 > > > Repository: aurora > > > Description > --- > > The pruner runs on periodic basis and trims completed updates up to the > guranteed per job retention count. > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java > aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef > src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java > ebae58a04e8857c5f26d4b57c27dfcda9e14c82c > src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java > PRE-CREATION > src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java > c3abffe575e801cebec3572cf4aceac83a238b55 > src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java > 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 > > src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java > c583e085e0458835d51ebf740a3b5f01b428bb25 > > src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java > 66c91644677e7176ccf53dcfcf29a6792ec398bc > > src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml > 2a81a94f99f242bbe400d8428ad1f1ce38a06a86 > src/main/thrift/org/apache/aurora/gen/storage.thrift > 7e502450f06abb449d06af09cc59185c6a9a2963 > src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java > 011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee > > src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java > PRE-CREATION > > src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java > dbf0badbfcc19f40d9b9eeec22b7024d95a02884 > src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java > 68df0d542e41438c0844f76fc5b9ec6996a00e8d > > src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java > e35fe23f023f5accfb2270a3634c91bec657 > src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java > 6758b7b56e77882c67be2e39481ff76893ad1610 > > Diff: https://reviews.apache.org/r/26232/diff/ > > > Testing > --- > > ./gradlew -Pq build > > > Thanks, > > Maxim Khutornenko > >
