Re: Review Request 37365: Add a new UI page to show all tasks (active and completed) for a specific instance id.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/ --- (Updated Aug. 18, 2015, 9:23 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Changes --- rebase. Repository: aurora Description --- Add a new UI page to show all tasks (active and completed) for a specific instance id. N.B.: changes to controllers.js look significant, but it's 90% refactoring so that JobController and JobInstanceController can reuse common code. Diffs (updated) - src/main/python/apache/aurora/client/base.py dee4c28a26828912bec073c7e5213a03c9703eee src/main/resources/scheduler/assets/breadcrumb.html 13147935c3c28680d5decd1e03e40d9cf04aaed3 src/main/resources/scheduler/assets/css/app.css a4735efe58a09a46f2b95ebe98f219cb9e7ee27d src/main/resources/scheduler/assets/instance.html PRE-CREATION src/main/resources/scheduler/assets/job.html 942635a61132bb9a38a6faf3f8fe046e6bcabd95 src/main/resources/scheduler/assets/js/app.js b66409f628046c67b62c92a75cbeed20c09b4ec1 src/main/resources/scheduler/assets/js/controllers.js 04ea1cb6998af7c238eb2fd1a7d05a9663abc56c src/main/resources/scheduler/assets/js/services.js a514fa7da3c049e1f5a3e49263a82cf2c28d1b2d src/main/resources/scheduler/assets/taskInstance.html PRE-CREATION src/test/python/apache/aurora/client/cli/test_supdate.py 21bea702a6e6fe68652e2195c5d24b56b6477738 src/test/python/apache/aurora/client/test_base.py 1a560088279ac945cce14d02454e50b8483771e4 Diff: https://reviews.apache.org/r/37365/diff/ Testing --- - ./gradlew jsHint - ./pants test src/test/python/apache/aurora/client:base - Verified update page url in client was correct in Vagrant. - See attached screenshot for an example of what the instance page looks like. File Attachments Example of instance page in action. https://reviews.apache.org/media/uploaded/files/2015/08/11/2ab72f9d-9e60-4b86-b971-b6aba5465def__Screen_Shot_2015-08-11_at_4.53.07_PM.png Thanks, Joshua Cohen
Re: Review Request 37365: Add a new UI page to show all tasks (active and completed) for a specific instance id.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/#review95775 --- Ship it! Looks much better with the services change, thanks! Screenshot also LGTM. - David McLaughlin On Aug. 17, 2015, 9:55 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/ --- (Updated Aug. 17, 2015, 9:55 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Repository: aurora Description --- Add a new UI page to show all tasks (active and completed) for a specific instance id. N.B.: changes to controllers.js look significant, but it's 90% refactoring so that JobController and JobInstanceController can reuse common code. Diffs - src/main/python/apache/aurora/client/base.py dee4c28a26828912bec073c7e5213a03c9703eee src/main/resources/scheduler/assets/breadcrumb.html 13147935c3c28680d5decd1e03e40d9cf04aaed3 src/main/resources/scheduler/assets/css/app.css a4735efe58a09a46f2b95ebe98f219cb9e7ee27d src/main/resources/scheduler/assets/instance.html PRE-CREATION src/main/resources/scheduler/assets/job.html 942635a61132bb9a38a6faf3f8fe046e6bcabd95 src/main/resources/scheduler/assets/js/app.js b66409f628046c67b62c92a75cbeed20c09b4ec1 src/main/resources/scheduler/assets/js/controllers.js 04ea1cb6998af7c238eb2fd1a7d05a9663abc56c src/main/resources/scheduler/assets/js/services.js a514fa7da3c049e1f5a3e49263a82cf2c28d1b2d src/main/resources/scheduler/assets/taskInstance.html PRE-CREATION src/test/python/apache/aurora/client/cli/test_supdate.py 21bea702a6e6fe68652e2195c5d24b56b6477738 src/test/python/apache/aurora/client/test_base.py 1a560088279ac945cce14d02454e50b8483771e4 Diff: https://reviews.apache.org/r/37365/diff/ Testing --- - ./gradlew jsHint - ./pants test src/test/python/apache/aurora/client:base - Verified update page url in client was correct in Vagrant. - See attached screenshot for an example of what the instance page looks like. File Attachments Example of instance page in action. https://reviews.apache.org/media/uploaded/files/2015/08/11/2ab72f9d-9e60-4b86-b971-b6aba5465def__Screen_Shot_2015-08-11_at_4.53.07_PM.png Thanks, Joshua Cohen
Re: Review Request 37365: Add a new UI page to show all tasks (active and completed) for a specific instance id.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/#review95669 --- Ship it! Master (22f9cbb) is green with this patch. ./build-support/jenkins/build.sh I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On Aug. 17, 2015, 9:55 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/ --- (Updated Aug. 17, 2015, 9:55 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Repository: aurora Description --- Add a new UI page to show all tasks (active and completed) for a specific instance id. N.B.: changes to controllers.js look significant, but it's 90% refactoring so that JobController and JobInstanceController can reuse common code. Diffs - src/main/python/apache/aurora/client/base.py dee4c28a26828912bec073c7e5213a03c9703eee src/main/resources/scheduler/assets/breadcrumb.html 13147935c3c28680d5decd1e03e40d9cf04aaed3 src/main/resources/scheduler/assets/css/app.css a4735efe58a09a46f2b95ebe98f219cb9e7ee27d src/main/resources/scheduler/assets/instance.html PRE-CREATION src/main/resources/scheduler/assets/job.html 942635a61132bb9a38a6faf3f8fe046e6bcabd95 src/main/resources/scheduler/assets/js/app.js b66409f628046c67b62c92a75cbeed20c09b4ec1 src/main/resources/scheduler/assets/js/controllers.js 04ea1cb6998af7c238eb2fd1a7d05a9663abc56c src/main/resources/scheduler/assets/js/services.js a514fa7da3c049e1f5a3e49263a82cf2c28d1b2d src/main/resources/scheduler/assets/taskInstance.html PRE-CREATION src/test/python/apache/aurora/client/cli/test_supdate.py 21bea702a6e6fe68652e2195c5d24b56b6477738 src/test/python/apache/aurora/client/test_base.py 1a560088279ac945cce14d02454e50b8483771e4 Diff: https://reviews.apache.org/r/37365/diff/ Testing --- - ./gradlew jsHint - ./pants test src/test/python/apache/aurora/client:base - Verified update page url in client was correct in Vagrant. - See attached screenshot for an example of what the instance page looks like. File Attachments Example of instance page in action. https://reviews.apache.org/media/uploaded/files/2015/08/11/2ab72f9d-9e60-4b86-b971-b6aba5465def__Screen_Shot_2015-08-11_at_4.53.07_PM.png Thanks, Joshua Cohen
Re: Review Request 37365: Add a new UI page to show all tasks (active and completed) for a specific instance id.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/ --- (Updated Aug. 14, 2015, 2:44 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Changes --- Removing binding `$scope` and `taskUtil` to `this`. Also fix one more link to the update page. Repository: aurora Description --- Add a new UI page to show all tasks (active and completed) for a specific instance id. N.B.: changes to controllers.js look significant, but it's 90% refactoring so that JobController and JobInstanceController can reuse common code. Diffs (updated) - src/main/python/apache/aurora/client/base.py dee4c28a26828912bec073c7e5213a03c9703eee src/main/resources/scheduler/assets/breadcrumb.html 13147935c3c28680d5decd1e03e40d9cf04aaed3 src/main/resources/scheduler/assets/css/app.css a4735efe58a09a46f2b95ebe98f219cb9e7ee27d src/main/resources/scheduler/assets/instance.html PRE-CREATION src/main/resources/scheduler/assets/job.html 942635a61132bb9a38a6faf3f8fe046e6bcabd95 src/main/resources/scheduler/assets/js/app.js b66409f628046c67b62c92a75cbeed20c09b4ec1 src/main/resources/scheduler/assets/js/controllers.js 04ea1cb6998af7c238eb2fd1a7d05a9663abc56c src/main/resources/scheduler/assets/js/services.js a514fa7da3c049e1f5a3e49263a82cf2c28d1b2d src/main/resources/scheduler/assets/taskInstance.html PRE-CREATION src/test/python/apache/aurora/client/cli/test_supdate.py 21bea702a6e6fe68652e2195c5d24b56b6477738 src/test/python/apache/aurora/client/test_base.py 1a560088279ac945cce14d02454e50b8483771e4 Diff: https://reviews.apache.org/r/37365/diff/ Testing --- - ./gradlew jsHint - ./pants test src/test/python/apache/aurora/client:base - Verified update page url in client was correct in Vagrant. - See attached screenshot for an example of what the instance page looks like. File Attachments Example of instance page in action. https://reviews.apache.org/media/uploaded/files/2015/08/11/2ab72f9d-9e60-4b86-b971-b6aba5465def__Screen_Shot_2015-08-11_at_4.53.07_PM.png Thanks, Joshua Cohen
Re: Review Request 37365: Add a new UI page to show all tasks (active and completed) for a specific instance id.
On Aug. 13, 2015, 9:48 p.m., David McLaughlin wrote: src/main/resources/scheduler/assets/js/controllers.js, line 334 https://reviews.apache.org/r/37365/diff/1/?file=1037854#file1037854line334 Why bind to this? Leftover cruft from an earlier implementation (wherein I had put the refactored methods on the prototype rather than within the constructor). I've removed it. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/#review95338 --- On Aug. 14, 2015, 2:44 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/ --- (Updated Aug. 14, 2015, 2:44 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Repository: aurora Description --- Add a new UI page to show all tasks (active and completed) for a specific instance id. N.B.: changes to controllers.js look significant, but it's 90% refactoring so that JobController and JobInstanceController can reuse common code. Diffs - src/main/python/apache/aurora/client/base.py dee4c28a26828912bec073c7e5213a03c9703eee src/main/resources/scheduler/assets/breadcrumb.html 13147935c3c28680d5decd1e03e40d9cf04aaed3 src/main/resources/scheduler/assets/css/app.css a4735efe58a09a46f2b95ebe98f219cb9e7ee27d src/main/resources/scheduler/assets/instance.html PRE-CREATION src/main/resources/scheduler/assets/job.html 942635a61132bb9a38a6faf3f8fe046e6bcabd95 src/main/resources/scheduler/assets/js/app.js b66409f628046c67b62c92a75cbeed20c09b4ec1 src/main/resources/scheduler/assets/js/controllers.js 04ea1cb6998af7c238eb2fd1a7d05a9663abc56c src/main/resources/scheduler/assets/js/services.js a514fa7da3c049e1f5a3e49263a82cf2c28d1b2d src/main/resources/scheduler/assets/taskInstance.html PRE-CREATION src/test/python/apache/aurora/client/cli/test_supdate.py 21bea702a6e6fe68652e2195c5d24b56b6477738 src/test/python/apache/aurora/client/test_base.py 1a560088279ac945cce14d02454e50b8483771e4 Diff: https://reviews.apache.org/r/37365/diff/ Testing --- - ./gradlew jsHint - ./pants test src/test/python/apache/aurora/client:base - Verified update page url in client was correct in Vagrant. - See attached screenshot for an example of what the instance page looks like. File Attachments Example of instance page in action. https://reviews.apache.org/media/uploaded/files/2015/08/11/2ab72f9d-9e60-4b86-b971-b6aba5465def__Screen_Shot_2015-08-11_at_4.53.07_PM.png Thanks, Joshua Cohen
Re: Review Request 37365: Add a new UI page to show all tasks (active and completed) for a specific instance id.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/#review95425 --- Ship it! Master (76d5a49) is green with this patch. ./build-support/jenkins/build.sh I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On Aug. 14, 2015, 2:44 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/ --- (Updated Aug. 14, 2015, 2:44 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Repository: aurora Description --- Add a new UI page to show all tasks (active and completed) for a specific instance id. N.B.: changes to controllers.js look significant, but it's 90% refactoring so that JobController and JobInstanceController can reuse common code. Diffs - src/main/python/apache/aurora/client/base.py dee4c28a26828912bec073c7e5213a03c9703eee src/main/resources/scheduler/assets/breadcrumb.html 13147935c3c28680d5decd1e03e40d9cf04aaed3 src/main/resources/scheduler/assets/css/app.css a4735efe58a09a46f2b95ebe98f219cb9e7ee27d src/main/resources/scheduler/assets/instance.html PRE-CREATION src/main/resources/scheduler/assets/job.html 942635a61132bb9a38a6faf3f8fe046e6bcabd95 src/main/resources/scheduler/assets/js/app.js b66409f628046c67b62c92a75cbeed20c09b4ec1 src/main/resources/scheduler/assets/js/controllers.js 04ea1cb6998af7c238eb2fd1a7d05a9663abc56c src/main/resources/scheduler/assets/js/services.js a514fa7da3c049e1f5a3e49263a82cf2c28d1b2d src/main/resources/scheduler/assets/taskInstance.html PRE-CREATION src/test/python/apache/aurora/client/cli/test_supdate.py 21bea702a6e6fe68652e2195c5d24b56b6477738 src/test/python/apache/aurora/client/test_base.py 1a560088279ac945cce14d02454e50b8483771e4 Diff: https://reviews.apache.org/r/37365/diff/ Testing --- - ./gradlew jsHint - ./pants test src/test/python/apache/aurora/client:base - Verified update page url in client was correct in Vagrant. - See attached screenshot for an example of what the instance page looks like. File Attachments Example of instance page in action. https://reviews.apache.org/media/uploaded/files/2015/08/11/2ab72f9d-9e60-4b86-b971-b6aba5465def__Screen_Shot_2015-08-11_at_4.53.07_PM.png Thanks, Joshua Cohen
Re: Review Request 37365: Add a new UI page to show all tasks (active and completed) for a specific instance id.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/#review95465 --- Ship it! Master (76d5a49) is green with this patch. ./build-support/jenkins/build.sh I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On Aug. 14, 2015, 7:17 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/ --- (Updated Aug. 14, 2015, 7:17 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Repository: aurora Description --- Add a new UI page to show all tasks (active and completed) for a specific instance id. N.B.: changes to controllers.js look significant, but it's 90% refactoring so that JobController and JobInstanceController can reuse common code. Diffs - src/main/python/apache/aurora/client/base.py dee4c28a26828912bec073c7e5213a03c9703eee src/main/resources/scheduler/assets/breadcrumb.html 13147935c3c28680d5decd1e03e40d9cf04aaed3 src/main/resources/scheduler/assets/css/app.css a4735efe58a09a46f2b95ebe98f219cb9e7ee27d src/main/resources/scheduler/assets/instance.html PRE-CREATION src/main/resources/scheduler/assets/job.html 942635a61132bb9a38a6faf3f8fe046e6bcabd95 src/main/resources/scheduler/assets/js/app.js b66409f628046c67b62c92a75cbeed20c09b4ec1 src/main/resources/scheduler/assets/js/controllers.js 04ea1cb6998af7c238eb2fd1a7d05a9663abc56c src/main/resources/scheduler/assets/js/services.js a514fa7da3c049e1f5a3e49263a82cf2c28d1b2d src/main/resources/scheduler/assets/taskInstance.html PRE-CREATION src/test/python/apache/aurora/client/cli/test_supdate.py 21bea702a6e6fe68652e2195c5d24b56b6477738 src/test/python/apache/aurora/client/test_base.py 1a560088279ac945cce14d02454e50b8483771e4 Diff: https://reviews.apache.org/r/37365/diff/ Testing --- - ./gradlew jsHint - ./pants test src/test/python/apache/aurora/client:base - Verified update page url in client was correct in Vagrant. - See attached screenshot for an example of what the instance page looks like. File Attachments Example of instance page in action. https://reviews.apache.org/media/uploaded/files/2015/08/11/2ab72f9d-9e60-4b86-b971-b6aba5465def__Screen_Shot_2015-08-11_at_4.53.07_PM.png Thanks, Joshua Cohen
Re: Review Request 37365: Add a new UI page to show all tasks (active and completed) for a specific instance id.
On Aug. 14, 2015, 3:54 p.m., David McLaughlin wrote: src/main/resources/scheduler/assets/js/controllers.js, line 606 https://reviews.apache.org/r/37365/diff/7/?file=1040096#file1040096line606 You can just do BaseJobController($scope, ...) now? Here and in other invocations. Joshua Cohen wrote: This is pretty standard mechanism for inheritance in JS. We could remove the .call invocation, but then if something changed in the future that *did* bind to `this` in the parent it wouldn't work properly. I'd prefer to keep it as is. David McLaughlin wrote: So maybe I don't understand why you're using classical inheritance here then, over a vanilla function? Joshua Cohen wrote: I guess it just made more sense conceptually, but maybe that's because I've been spending way too much time writing java lately ;). Do you think a vanilla function is easier to understand in this context? David McLaughlin wrote: Yeah I mean if we keep it this way, it seems to be advocating an approach where any simple function needs to be explicit about what 'this' is when invoked, just in case it's ever used? Does that make sense? On further reflection, I *am* using inheritance to expose addColumn and taskIdColumn to the instance controller, so binding to `this` is necessary (see line ~620 in controllers.js). The alternative would be to make them loose functions/variables, which is unappealing since it breaks encapsulation. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/#review95427 --- On Aug. 14, 2015, 7:17 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/ --- (Updated Aug. 14, 2015, 7:17 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Repository: aurora Description --- Add a new UI page to show all tasks (active and completed) for a specific instance id. N.B.: changes to controllers.js look significant, but it's 90% refactoring so that JobController and JobInstanceController can reuse common code. Diffs - src/main/python/apache/aurora/client/base.py dee4c28a26828912bec073c7e5213a03c9703eee src/main/resources/scheduler/assets/breadcrumb.html 13147935c3c28680d5decd1e03e40d9cf04aaed3 src/main/resources/scheduler/assets/css/app.css a4735efe58a09a46f2b95ebe98f219cb9e7ee27d src/main/resources/scheduler/assets/instance.html PRE-CREATION src/main/resources/scheduler/assets/job.html 942635a61132bb9a38a6faf3f8fe046e6bcabd95 src/main/resources/scheduler/assets/js/app.js b66409f628046c67b62c92a75cbeed20c09b4ec1 src/main/resources/scheduler/assets/js/controllers.js 04ea1cb6998af7c238eb2fd1a7d05a9663abc56c src/main/resources/scheduler/assets/js/services.js a514fa7da3c049e1f5a3e49263a82cf2c28d1b2d src/main/resources/scheduler/assets/taskInstance.html PRE-CREATION src/test/python/apache/aurora/client/cli/test_supdate.py 21bea702a6e6fe68652e2195c5d24b56b6477738 src/test/python/apache/aurora/client/test_base.py 1a560088279ac945cce14d02454e50b8483771e4 Diff: https://reviews.apache.org/r/37365/diff/ Testing --- - ./gradlew jsHint - ./pants test src/test/python/apache/aurora/client:base - Verified update page url in client was correct in Vagrant. - See attached screenshot for an example of what the instance page looks like. File Attachments Example of instance page in action. https://reviews.apache.org/media/uploaded/files/2015/08/11/2ab72f9d-9e60-4b86-b971-b6aba5465def__Screen_Shot_2015-08-11_at_4.53.07_PM.png Thanks, Joshua Cohen
Re: Review Request 37365: Add a new UI page to show all tasks (active and completed) for a specific instance id.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/ --- (Updated Aug. 14, 2015, 7:17 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Changes --- Redirect to update page if instance id looks like a guid. Repository: aurora Description --- Add a new UI page to show all tasks (active and completed) for a specific instance id. N.B.: changes to controllers.js look significant, but it's 90% refactoring so that JobController and JobInstanceController can reuse common code. Diffs (updated) - src/main/python/apache/aurora/client/base.py dee4c28a26828912bec073c7e5213a03c9703eee src/main/resources/scheduler/assets/breadcrumb.html 13147935c3c28680d5decd1e03e40d9cf04aaed3 src/main/resources/scheduler/assets/css/app.css a4735efe58a09a46f2b95ebe98f219cb9e7ee27d src/main/resources/scheduler/assets/instance.html PRE-CREATION src/main/resources/scheduler/assets/job.html 942635a61132bb9a38a6faf3f8fe046e6bcabd95 src/main/resources/scheduler/assets/js/app.js b66409f628046c67b62c92a75cbeed20c09b4ec1 src/main/resources/scheduler/assets/js/controllers.js 04ea1cb6998af7c238eb2fd1a7d05a9663abc56c src/main/resources/scheduler/assets/js/services.js a514fa7da3c049e1f5a3e49263a82cf2c28d1b2d src/main/resources/scheduler/assets/taskInstance.html PRE-CREATION src/test/python/apache/aurora/client/cli/test_supdate.py 21bea702a6e6fe68652e2195c5d24b56b6477738 src/test/python/apache/aurora/client/test_base.py 1a560088279ac945cce14d02454e50b8483771e4 Diff: https://reviews.apache.org/r/37365/diff/ Testing --- - ./gradlew jsHint - ./pants test src/test/python/apache/aurora/client:base - Verified update page url in client was correct in Vagrant. - See attached screenshot for an example of what the instance page looks like. File Attachments Example of instance page in action. https://reviews.apache.org/media/uploaded/files/2015/08/11/2ab72f9d-9e60-4b86-b971-b6aba5465def__Screen_Shot_2015-08-11_at_4.53.07_PM.png Thanks, Joshua Cohen
Re: Review Request 37365: Add a new UI page to show all tasks (active and completed) for a specific instance id.
On Aug. 14, 2015, 3:54 p.m., David McLaughlin wrote: src/main/resources/scheduler/assets/js/controllers.js, line 606 https://reviews.apache.org/r/37365/diff/7/?file=1040096#file1040096line606 You can just do BaseJobController($scope, ...) now? Here and in other invocations. Joshua Cohen wrote: This is pretty standard mechanism for inheritance in JS. We could remove the .call invocation, but then if something changed in the future that *did* bind to `this` in the parent it wouldn't work properly. I'd prefer to keep it as is. David McLaughlin wrote: So maybe I don't understand why you're using classical inheritance here then, over a vanilla function? I guess it just made more sense conceptually, but maybe that's because I've been spending way too much time writing java lately ;). Do you think a vanilla function is easier to understand in this context? - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/#review95427 --- On Aug. 14, 2015, 2:44 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/ --- (Updated Aug. 14, 2015, 2:44 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Repository: aurora Description --- Add a new UI page to show all tasks (active and completed) for a specific instance id. N.B.: changes to controllers.js look significant, but it's 90% refactoring so that JobController and JobInstanceController can reuse common code. Diffs - src/main/python/apache/aurora/client/base.py dee4c28a26828912bec073c7e5213a03c9703eee src/main/resources/scheduler/assets/breadcrumb.html 13147935c3c28680d5decd1e03e40d9cf04aaed3 src/main/resources/scheduler/assets/css/app.css a4735efe58a09a46f2b95ebe98f219cb9e7ee27d src/main/resources/scheduler/assets/instance.html PRE-CREATION src/main/resources/scheduler/assets/job.html 942635a61132bb9a38a6faf3f8fe046e6bcabd95 src/main/resources/scheduler/assets/js/app.js b66409f628046c67b62c92a75cbeed20c09b4ec1 src/main/resources/scheduler/assets/js/controllers.js 04ea1cb6998af7c238eb2fd1a7d05a9663abc56c src/main/resources/scheduler/assets/js/services.js a514fa7da3c049e1f5a3e49263a82cf2c28d1b2d src/main/resources/scheduler/assets/taskInstance.html PRE-CREATION src/test/python/apache/aurora/client/cli/test_supdate.py 21bea702a6e6fe68652e2195c5d24b56b6477738 src/test/python/apache/aurora/client/test_base.py 1a560088279ac945cce14d02454e50b8483771e4 Diff: https://reviews.apache.org/r/37365/diff/ Testing --- - ./gradlew jsHint - ./pants test src/test/python/apache/aurora/client:base - Verified update page url in client was correct in Vagrant. - See attached screenshot for an example of what the instance page looks like. File Attachments Example of instance page in action. https://reviews.apache.org/media/uploaded/files/2015/08/11/2ab72f9d-9e60-4b86-b971-b6aba5465def__Screen_Shot_2015-08-11_at_4.53.07_PM.png Thanks, Joshua Cohen
Re: Review Request 37365: Add a new UI page to show all tasks (active and completed) for a specific instance id.
On Aug. 14, 2015, 3:54 p.m., David McLaughlin wrote: src/main/resources/scheduler/assets/js/controllers.js, line 606 https://reviews.apache.org/r/37365/diff/7/?file=1040096#file1040096line606 You can just do BaseJobController($scope, ...) now? Here and in other invocations. Joshua Cohen wrote: This is pretty standard mechanism for inheritance in JS. We could remove the .call invocation, but then if something changed in the future that *did* bind to `this` in the parent it wouldn't work properly. I'd prefer to keep it as is. David McLaughlin wrote: So maybe I don't understand why you're using classical inheritance here then, over a vanilla function? Joshua Cohen wrote: I guess it just made more sense conceptually, but maybe that's because I've been spending way too much time writing java lately ;). Do you think a vanilla function is easier to understand in this context? Yeah I mean if we keep it this way, it seems to be advocating an approach where any simple function needs to be explicit about what 'this' is when invoked, just in case it's ever used? Does that make sense? - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/#review95427 --- On Aug. 14, 2015, 2:44 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/ --- (Updated Aug. 14, 2015, 2:44 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Repository: aurora Description --- Add a new UI page to show all tasks (active and completed) for a specific instance id. N.B.: changes to controllers.js look significant, but it's 90% refactoring so that JobController and JobInstanceController can reuse common code. Diffs - src/main/python/apache/aurora/client/base.py dee4c28a26828912bec073c7e5213a03c9703eee src/main/resources/scheduler/assets/breadcrumb.html 13147935c3c28680d5decd1e03e40d9cf04aaed3 src/main/resources/scheduler/assets/css/app.css a4735efe58a09a46f2b95ebe98f219cb9e7ee27d src/main/resources/scheduler/assets/instance.html PRE-CREATION src/main/resources/scheduler/assets/job.html 942635a61132bb9a38a6faf3f8fe046e6bcabd95 src/main/resources/scheduler/assets/js/app.js b66409f628046c67b62c92a75cbeed20c09b4ec1 src/main/resources/scheduler/assets/js/controllers.js 04ea1cb6998af7c238eb2fd1a7d05a9663abc56c src/main/resources/scheduler/assets/js/services.js a514fa7da3c049e1f5a3e49263a82cf2c28d1b2d src/main/resources/scheduler/assets/taskInstance.html PRE-CREATION src/test/python/apache/aurora/client/cli/test_supdate.py 21bea702a6e6fe68652e2195c5d24b56b6477738 src/test/python/apache/aurora/client/test_base.py 1a560088279ac945cce14d02454e50b8483771e4 Diff: https://reviews.apache.org/r/37365/diff/ Testing --- - ./gradlew jsHint - ./pants test src/test/python/apache/aurora/client:base - Verified update page url in client was correct in Vagrant. - See attached screenshot for an example of what the instance page looks like. File Attachments Example of instance page in action. https://reviews.apache.org/media/uploaded/files/2015/08/11/2ab72f9d-9e60-4b86-b971-b6aba5465def__Screen_Shot_2015-08-11_at_4.53.07_PM.png Thanks, Joshua Cohen
Re: Review Request 37365: Add a new UI page to show all tasks (active and completed) for a specific instance id.
On Aug. 14, 2015, 3:54 p.m., David McLaughlin wrote: src/main/resources/scheduler/assets/js/controllers.js, line 606 https://reviews.apache.org/r/37365/diff/7/?file=1040096#file1040096line606 You can just do BaseJobController($scope, ...) now? Here and in other invocations. Joshua Cohen wrote: This is pretty standard mechanism for inheritance in JS. We could remove the .call invocation, but then if something changed in the future that *did* bind to `this` in the parent it wouldn't work properly. I'd prefer to keep it as is. David McLaughlin wrote: So maybe I don't understand why you're using classical inheritance here then, over a vanilla function? Joshua Cohen wrote: I guess it just made more sense conceptually, but maybe that's because I've been spending way too much time writing java lately ;). Do you think a vanilla function is easier to understand in this context? David McLaughlin wrote: Yeah I mean if we keep it this way, it seems to be advocating an approach where any simple function needs to be explicit about what 'this' is when invoked, just in case it's ever used? Does that make sense? Joshua Cohen wrote: On further reflection, I *am* using inheritance to expose addColumn and taskIdColumn to the instance controller, so binding to `this` is necessary (see line ~620 in controllers.js). The alternative would be to make them loose functions/variables, which is unappealing since it breaks encapsulation. David McLaughlin wrote: On a quick glance it seems like those two functions don't rely on state pulled into the closure. Can't you just move them into services since they are effectively 'static' functions? Oh! Maybe! I'm not so good at Angular. Maybe the whole bit of shared functionality between the two controllers should really be a service! - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/#review95427 --- On Aug. 14, 2015, 7:17 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/ --- (Updated Aug. 14, 2015, 7:17 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Repository: aurora Description --- Add a new UI page to show all tasks (active and completed) for a specific instance id. N.B.: changes to controllers.js look significant, but it's 90% refactoring so that JobController and JobInstanceController can reuse common code. Diffs - src/main/python/apache/aurora/client/base.py dee4c28a26828912bec073c7e5213a03c9703eee src/main/resources/scheduler/assets/breadcrumb.html 13147935c3c28680d5decd1e03e40d9cf04aaed3 src/main/resources/scheduler/assets/css/app.css a4735efe58a09a46f2b95ebe98f219cb9e7ee27d src/main/resources/scheduler/assets/instance.html PRE-CREATION src/main/resources/scheduler/assets/job.html 942635a61132bb9a38a6faf3f8fe046e6bcabd95 src/main/resources/scheduler/assets/js/app.js b66409f628046c67b62c92a75cbeed20c09b4ec1 src/main/resources/scheduler/assets/js/controllers.js 04ea1cb6998af7c238eb2fd1a7d05a9663abc56c src/main/resources/scheduler/assets/js/services.js a514fa7da3c049e1f5a3e49263a82cf2c28d1b2d src/main/resources/scheduler/assets/taskInstance.html PRE-CREATION src/test/python/apache/aurora/client/cli/test_supdate.py 21bea702a6e6fe68652e2195c5d24b56b6477738 src/test/python/apache/aurora/client/test_base.py 1a560088279ac945cce14d02454e50b8483771e4 Diff: https://reviews.apache.org/r/37365/diff/ Testing --- - ./gradlew jsHint - ./pants test src/test/python/apache/aurora/client:base - Verified update page url in client was correct in Vagrant. - See attached screenshot for an example of what the instance page looks like. File Attachments Example of instance page in action. https://reviews.apache.org/media/uploaded/files/2015/08/11/2ab72f9d-9e60-4b86-b971-b6aba5465def__Screen_Shot_2015-08-11_at_4.53.07_PM.png Thanks, Joshua Cohen
Re: Review Request 37365: Add a new UI page to show all tasks (active and completed) for a specific instance id.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/#review95427 --- src/main/resources/scheduler/assets/js/app.js (lines 35 - 38) https://reviews.apache.org/r/37365/#comment150366 I feel like changing our URLs like this needs a deprecation path. This would break tooling around Aurora that assumes how to construct update URLs, as seen by the fix you had to apply to the client code. src/main/resources/scheduler/assets/js/controllers.js (line 521) https://reviews.apache.org/r/37365/#comment150365 You can just do BaseJobController($scope, ...) now? Here and in other invocations. - David McLaughlin On Aug. 14, 2015, 2:44 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/ --- (Updated Aug. 14, 2015, 2:44 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Repository: aurora Description --- Add a new UI page to show all tasks (active and completed) for a specific instance id. N.B.: changes to controllers.js look significant, but it's 90% refactoring so that JobController and JobInstanceController can reuse common code. Diffs - src/main/python/apache/aurora/client/base.py dee4c28a26828912bec073c7e5213a03c9703eee src/main/resources/scheduler/assets/breadcrumb.html 13147935c3c28680d5decd1e03e40d9cf04aaed3 src/main/resources/scheduler/assets/css/app.css a4735efe58a09a46f2b95ebe98f219cb9e7ee27d src/main/resources/scheduler/assets/instance.html PRE-CREATION src/main/resources/scheduler/assets/job.html 942635a61132bb9a38a6faf3f8fe046e6bcabd95 src/main/resources/scheduler/assets/js/app.js b66409f628046c67b62c92a75cbeed20c09b4ec1 src/main/resources/scheduler/assets/js/controllers.js 04ea1cb6998af7c238eb2fd1a7d05a9663abc56c src/main/resources/scheduler/assets/js/services.js a514fa7da3c049e1f5a3e49263a82cf2c28d1b2d src/main/resources/scheduler/assets/taskInstance.html PRE-CREATION src/test/python/apache/aurora/client/cli/test_supdate.py 21bea702a6e6fe68652e2195c5d24b56b6477738 src/test/python/apache/aurora/client/test_base.py 1a560088279ac945cce14d02454e50b8483771e4 Diff: https://reviews.apache.org/r/37365/diff/ Testing --- - ./gradlew jsHint - ./pants test src/test/python/apache/aurora/client:base - Verified update page url in client was correct in Vagrant. - See attached screenshot for an example of what the instance page looks like. File Attachments Example of instance page in action. https://reviews.apache.org/media/uploaded/files/2015/08/11/2ab72f9d-9e60-4b86-b971-b6aba5465def__Screen_Shot_2015-08-11_at_4.53.07_PM.png Thanks, Joshua Cohen
Re: Review Request 37365: Add a new UI page to show all tasks (active and completed) for a specific instance id.
On Aug. 14, 2015, 3:54 p.m., David McLaughlin wrote: src/main/resources/scheduler/assets/js/app.js, lines 35-38 https://reviews.apache.org/r/37365/diff/7/?file=1040095#file1040095line35 I feel like changing our URLs like this needs a deprecation path. This would break tooling around Aurora that assumes how to construct update URLs, as seen by the fix you had to apply to the client code. Yeah, I realize this couples a scheduler and client release. I could try and detect instance ids that seem like guids and redirect? That way as long as the scheduler is released before the client it should work in both scenarios. Do you think that would be sufficient? On Aug. 14, 2015, 3:54 p.m., David McLaughlin wrote: src/main/resources/scheduler/assets/js/controllers.js, line 606 https://reviews.apache.org/r/37365/diff/7/?file=1040096#file1040096line606 You can just do BaseJobController($scope, ...) now? Here and in other invocations. This is pretty standard mechanism for inheritance in JS. We could remove the .call invocation, but then if something changed in the future that *did* bind to `this` in the parent it wouldn't work properly. I'd prefer to keep it as is. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/#review95427 --- On Aug. 14, 2015, 2:44 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/ --- (Updated Aug. 14, 2015, 2:44 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Repository: aurora Description --- Add a new UI page to show all tasks (active and completed) for a specific instance id. N.B.: changes to controllers.js look significant, but it's 90% refactoring so that JobController and JobInstanceController can reuse common code. Diffs - src/main/python/apache/aurora/client/base.py dee4c28a26828912bec073c7e5213a03c9703eee src/main/resources/scheduler/assets/breadcrumb.html 13147935c3c28680d5decd1e03e40d9cf04aaed3 src/main/resources/scheduler/assets/css/app.css a4735efe58a09a46f2b95ebe98f219cb9e7ee27d src/main/resources/scheduler/assets/instance.html PRE-CREATION src/main/resources/scheduler/assets/job.html 942635a61132bb9a38a6faf3f8fe046e6bcabd95 src/main/resources/scheduler/assets/js/app.js b66409f628046c67b62c92a75cbeed20c09b4ec1 src/main/resources/scheduler/assets/js/controllers.js 04ea1cb6998af7c238eb2fd1a7d05a9663abc56c src/main/resources/scheduler/assets/js/services.js a514fa7da3c049e1f5a3e49263a82cf2c28d1b2d src/main/resources/scheduler/assets/taskInstance.html PRE-CREATION src/test/python/apache/aurora/client/cli/test_supdate.py 21bea702a6e6fe68652e2195c5d24b56b6477738 src/test/python/apache/aurora/client/test_base.py 1a560088279ac945cce14d02454e50b8483771e4 Diff: https://reviews.apache.org/r/37365/diff/ Testing --- - ./gradlew jsHint - ./pants test src/test/python/apache/aurora/client:base - Verified update page url in client was correct in Vagrant. - See attached screenshot for an example of what the instance page looks like. File Attachments Example of instance page in action. https://reviews.apache.org/media/uploaded/files/2015/08/11/2ab72f9d-9e60-4b86-b971-b6aba5465def__Screen_Shot_2015-08-11_at_4.53.07_PM.png Thanks, Joshua Cohen
Re: Review Request 37365: Add a new UI page to show all tasks (active and completed) for a specific instance id.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/#review95338 --- src/main/resources/scheduler/assets/js/controllers.js (line 334) https://reviews.apache.org/r/37365/#comment150249 Why bind to this? - David McLaughlin On Aug. 13, 2015, 6:42 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/ --- (Updated Aug. 13, 2015, 6:42 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Repository: aurora Description --- Add a new UI page to show all tasks (active and completed) for a specific instance id. N.B.: changes to controllers.js look significant, but it's 90% refactoring so that JobController and JobInstanceController can reuse common code. Diffs - src/main/python/apache/aurora/client/base.py dee4c28a26828912bec073c7e5213a03c9703eee src/main/resources/scheduler/assets/breadcrumb.html 13147935c3c28680d5decd1e03e40d9cf04aaed3 src/main/resources/scheduler/assets/css/app.css a4735efe58a09a46f2b95ebe98f219cb9e7ee27d src/main/resources/scheduler/assets/instance.html PRE-CREATION src/main/resources/scheduler/assets/job.html 942635a61132bb9a38a6faf3f8fe046e6bcabd95 src/main/resources/scheduler/assets/js/app.js b66409f628046c67b62c92a75cbeed20c09b4ec1 src/main/resources/scheduler/assets/js/controllers.js 04ea1cb6998af7c238eb2fd1a7d05a9663abc56c src/main/resources/scheduler/assets/js/services.js a514fa7da3c049e1f5a3e49263a82cf2c28d1b2d src/main/resources/scheduler/assets/taskInstance.html PRE-CREATION src/test/python/apache/aurora/client/cli/test_supdate.py 21bea702a6e6fe68652e2195c5d24b56b6477738 src/test/python/apache/aurora/client/test_base.py 1a560088279ac945cce14d02454e50b8483771e4 Diff: https://reviews.apache.org/r/37365/diff/ Testing --- - ./gradlew jsHint - ./pants test src/test/python/apache/aurora/client:base - Verified update page url in client was correct in Vagrant. - See attached screenshot for an example of what the instance page looks like. File Attachments Example of instance page in action. https://reviews.apache.org/media/uploaded/files/2015/08/11/2ab72f9d-9e60-4b86-b971-b6aba5465def__Screen_Shot_2015-08-11_at_4.53.07_PM.png Thanks, Joshua Cohen
Re: Review Request 37365: Add a new UI page to show all tasks (active and completed) for a specific instance id.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/#review95319 --- Ship it! Master (887ffd2) is green with this patch. ./build-support/jenkins/build.sh I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On Aug. 13, 2015, 6:42 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/ --- (Updated Aug. 13, 2015, 6:42 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Repository: aurora Description --- Add a new UI page to show all tasks (active and completed) for a specific instance id. N.B.: changes to controllers.js look significant, but it's 90% refactoring so that JobController and JobInstanceController can reuse common code. Diffs - src/main/python/apache/aurora/client/base.py dee4c28a26828912bec073c7e5213a03c9703eee src/main/resources/scheduler/assets/breadcrumb.html 13147935c3c28680d5decd1e03e40d9cf04aaed3 src/main/resources/scheduler/assets/css/app.css a4735efe58a09a46f2b95ebe98f219cb9e7ee27d src/main/resources/scheduler/assets/instance.html PRE-CREATION src/main/resources/scheduler/assets/job.html 942635a61132bb9a38a6faf3f8fe046e6bcabd95 src/main/resources/scheduler/assets/js/app.js b66409f628046c67b62c92a75cbeed20c09b4ec1 src/main/resources/scheduler/assets/js/controllers.js 04ea1cb6998af7c238eb2fd1a7d05a9663abc56c src/main/resources/scheduler/assets/js/services.js a514fa7da3c049e1f5a3e49263a82cf2c28d1b2d src/main/resources/scheduler/assets/taskInstance.html PRE-CREATION src/test/python/apache/aurora/client/cli/test_supdate.py 21bea702a6e6fe68652e2195c5d24b56b6477738 src/test/python/apache/aurora/client/test_base.py 1a560088279ac945cce14d02454e50b8483771e4 Diff: https://reviews.apache.org/r/37365/diff/ Testing --- - ./gradlew jsHint - ./pants test src/test/python/apache/aurora/client:base - Verified update page url in client was correct in Vagrant. - See attached screenshot for an example of what the instance page looks like. File Attachments Example of instance page in action. https://reviews.apache.org/media/uploaded/files/2015/08/11/2ab72f9d-9e60-4b86-b971-b6aba5465def__Screen_Shot_2015-08-11_at_4.53.07_PM.png Thanks, Joshua Cohen
Re: Review Request 37365: Add a new UI page to show all tasks (active and completed) for a specific instance id.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/#review95329 --- Ship it! Ship It! - Maxim Khutornenko On Aug. 13, 2015, 6:42 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/ --- (Updated Aug. 13, 2015, 6:42 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Repository: aurora Description --- Add a new UI page to show all tasks (active and completed) for a specific instance id. N.B.: changes to controllers.js look significant, but it's 90% refactoring so that JobController and JobInstanceController can reuse common code. Diffs - src/main/python/apache/aurora/client/base.py dee4c28a26828912bec073c7e5213a03c9703eee src/main/resources/scheduler/assets/breadcrumb.html 13147935c3c28680d5decd1e03e40d9cf04aaed3 src/main/resources/scheduler/assets/css/app.css a4735efe58a09a46f2b95ebe98f219cb9e7ee27d src/main/resources/scheduler/assets/instance.html PRE-CREATION src/main/resources/scheduler/assets/job.html 942635a61132bb9a38a6faf3f8fe046e6bcabd95 src/main/resources/scheduler/assets/js/app.js b66409f628046c67b62c92a75cbeed20c09b4ec1 src/main/resources/scheduler/assets/js/controllers.js 04ea1cb6998af7c238eb2fd1a7d05a9663abc56c src/main/resources/scheduler/assets/js/services.js a514fa7da3c049e1f5a3e49263a82cf2c28d1b2d src/main/resources/scheduler/assets/taskInstance.html PRE-CREATION src/test/python/apache/aurora/client/cli/test_supdate.py 21bea702a6e6fe68652e2195c5d24b56b6477738 src/test/python/apache/aurora/client/test_base.py 1a560088279ac945cce14d02454e50b8483771e4 Diff: https://reviews.apache.org/r/37365/diff/ Testing --- - ./gradlew jsHint - ./pants test src/test/python/apache/aurora/client:base - Verified update page url in client was correct in Vagrant. - See attached screenshot for an example of what the instance page looks like. File Attachments Example of instance page in action. https://reviews.apache.org/media/uploaded/files/2015/08/11/2ab72f9d-9e60-4b86-b971-b6aba5465def__Screen_Shot_2015-08-11_at_4.53.07_PM.png Thanks, Joshua Cohen
Re: Review Request 37365: Add a new UI page to show all tasks (active and completed) for a specific instance id.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/#review95094 --- Master (cbc42c4) is red with this patch. ./build-support/jenkins/build.sh src.test.python.apache.aurora.client.api.updater_util . SUCCESS src.test.python.apache.aurora.client.base . SUCCESS src.test.python.apache.aurora.client.binding_helper . SUCCESS src.test.python.apache.aurora.client.cli.api . SUCCESS src.test.python.apache.aurora.client.cli.client . SUCCESS src.test.python.apache.aurora.client.cli.command_hooks . SUCCESS src.test.python.apache.aurora.client.cli.config . SUCCESS src.test.python.apache.aurora.client.cli.context . SUCCESS src.test.python.apache.aurora.client.cli.cron . SUCCESS src.test.python.apache.aurora.client.cli.inspect . SUCCESS src.test.python.apache.aurora.client.cli.job . SUCCESS src.test.python.apache.aurora.client.cli.options . SUCCESS src.test.python.apache.aurora.client.cli.plugins . SUCCESS src.test.python.apache.aurora.client.cli.quota . SUCCESS src.test.python.apache.aurora.client.cli.sla . SUCCESS src.test.python.apache.aurora.client.cli.supdate . FAILURE src.test.python.apache.aurora.client.cli.task . SUCCESS src.test.python.apache.aurora.client.config . SUCCESS src.test.python.apache.aurora.client.hooks.non_hooked_api . SUCCESS src.test.python.apache.aurora.common.test_aurora_job_key . SUCCESS src.test.python.apache.aurora.common.test_cluster . SUCCESS src.test.python.apache.aurora.common.test_cluster_option . SUCCESS src.test.python.apache.aurora.common.test_clusters . SUCCESS src.test.python.apache.aurora.common.test_http_signaler . SUCCESS src.test.python.apache.aurora.common.test_pex_version . SUCCESS src.test.python.apache.aurora.common.test_shellify . SUCCESS src.test.python.apache.aurora.common.test_transport . SUCCESS src.test.python.apache.aurora.executor.common.path_detector . SUCCESS src.test.python.apache.aurora.executor.common.task_info . SUCCESS src.test.python.apache.thermos.cli.commands.commands . SUCCESS src.test.python.apache.thermos.cli.common . SUCCESS src.test.python.apache.thermos.cli.main . SUCCESS src.test.python.apache.thermos.common.test_pathspec . SUCCESS src.test.python.apache.thermos.core.test_runner_integration . SUCCESS src.test.python.apache.thermos.monitoring.test_disk . SUCCESS FAILURE [31m FAILURE[0m I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On Aug. 12, 2015, 3:29 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/ --- (Updated Aug. 12, 2015, 3:29 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko.
Re: Review Request 37365: Add a new UI page to show all tasks (active and completed) for a specific instance id.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/ --- (Updated Aug. 12, 2015, 3:49 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Changes --- Fix url in supdate tests. Repository: aurora Description --- Add a new UI page to show all tasks (active and completed) for a specific instance id. N.B.: changes to controllers.js look significant, but it's 90% refactoring so that JobController and JobInstanceController can reuse common code. Diffs (updated) - src/main/python/apache/aurora/client/base.py dee4c28a26828912bec073c7e5213a03c9703eee src/main/resources/scheduler/assets/breadcrumb.html 13147935c3c28680d5decd1e03e40d9cf04aaed3 src/main/resources/scheduler/assets/css/app.css a4735efe58a09a46f2b95ebe98f219cb9e7ee27d src/main/resources/scheduler/assets/instance.html PRE-CREATION src/main/resources/scheduler/assets/job.html 942635a61132bb9a38a6faf3f8fe046e6bcabd95 src/main/resources/scheduler/assets/js/app.js b66409f628046c67b62c92a75cbeed20c09b4ec1 src/main/resources/scheduler/assets/js/controllers.js 04ea1cb6998af7c238eb2fd1a7d05a9663abc56c src/main/resources/scheduler/assets/js/services.js a514fa7da3c049e1f5a3e49263a82cf2c28d1b2d src/main/resources/scheduler/assets/taskInstance.html PRE-CREATION src/test/python/apache/aurora/client/cli/test_supdate.py 21bea702a6e6fe68652e2195c5d24b56b6477738 src/test/python/apache/aurora/client/test_base.py 1a560088279ac945cce14d02454e50b8483771e4 Diff: https://reviews.apache.org/r/37365/diff/ Testing --- - ./gradlew jsHint - ./pants test src/test/python/apache/aurora/client:base - Verified update page url in client was correct in Vagrant. - See attached screenshot for an example of what the instance page looks like. File Attachments Example of instance page in action. https://reviews.apache.org/media/uploaded/files/2015/08/11/2ab72f9d-9e60-4b86-b971-b6aba5465def__Screen_Shot_2015-08-11_at_4.53.07_PM.png Thanks, Joshua Cohen
Re: Review Request 37365: Add a new UI page to show all tasks (active and completed) for a specific instance id.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/#review95098 --- Master (cbc42c4) is red with this patch. ./build-support/jenkins/build.sh Collecting twitter.common.util==0.3.0 (from twitter.common.app==0.3.0-twitter.checkstyle==0.1.0) Using cached twitter.common.util-0.3.0.tar.gz Collecting twitter.common.collections==0.3.0 (from twitter.common.app==0.3.0-twitter.checkstyle==0.1.0) Using cached twitter.common.collections-0.3.0.tar.gz Collecting smmap=0.8.5 (from gitdb=0.5.1-GitPython==0.3.2.RC1-twitter.checkstyle==0.1.0) Using cached smmap-0.9.0.tar.gz Collecting twitter.common.string==0.3.0 (from twitter.common.process==0.3.0-twitter.common.app==0.3.0-twitter.checkstyle==0.1.0) Using cached twitter.common.string-0.3.0.tar.gz Collecting twitter.common.options==0.3.0 (from twitter.common.log==0.3.0-twitter.common.app==0.3.0-twitter.checkstyle==0.1.0) Using cached twitter.common.options-0.3.0.tar.gz Collecting twitter.common.dirutil==0.3.0 (from twitter.common.log==0.3.0-twitter.common.app==0.3.0-twitter.checkstyle==0.1.0) Using cached twitter.common.dirutil-0.3.0.tar.gz Collecting twitter.common.contextutil==0.3.0 (from twitter.common.util==0.3.0-twitter.common.app==0.3.0-twitter.checkstyle==0.1.0) Using cached twitter.common.contextutil-0.3.0.tar.gz Collecting twitter.common.lang==0.3.0 (from twitter.common.collections==0.3.0-twitter.common.app==0.3.0-twitter.checkstyle==0.1.0) Using cached twitter.common.lang-0.3.0.tar.gz Installing collected packages: pyflakes, pep8, smmap, gitdb, GitPython, twitter.common.lang, twitter.common.string, twitter.common.process, twitter.common.options, twitter.common.dirutil, twitter.common.log, twitter.common.contextutil, twitter.common.util, twitter.common.collections, twitter.common.app, twitter.checkstyle Running setup.py install for pyflakes Running setup.py install for pep8 Running setup.py install for smmap Running setup.py install for gitdb Running setup.py install for GitPython Running setup.py install for twitter.common.lang Running setup.py install for twitter.common.string Running setup.py install for twitter.common.process Running setup.py install for twitter.common.options Running setup.py install for twitter.common.dirutil Running setup.py install for twitter.common.log Running setup.py install for twitter.common.contextutil Running setup.py install for twitter.common.util Running setup.py install for twitter.common.collections Running setup.py install for twitter.common.app Running setup.py install for twitter.checkstyle Successfully installed GitPython-0.3.2rc1 gitdb-0.6.4 pep8-1.4.5 pyflakes-0.7.2 smmap-0.9.0 twitter.checkstyle-0.1.0 twitter.common.app-0.3.0 twitter.common.collections-0.3.0 twitter.common.contextutil-0.3.0 twitter.common.dirutil-0.3.0 twitter.common.lang-0.3.0 twitter.common.log-0.3.0 twitter.common.options-0.3.0 twitter.common.process-0.3.0 twitter.common.string-0.3.0 twitter.common.util-0.3.0 E126:ERROR src/test/python/apache/aurora/client/cli/test_supdate.py:162 continuation line over-indented for hanging indent | ('http://something_or_other/scheduler/role/env/name/update/id') E126:ERROR src/test/python/apache/aurora/client/cli/test_supdate.py:188 continuation line over-indented for hanging indent | ('http://something_or_other/scheduler/role/env/name/update/id'), I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On Aug. 12, 2015, 3:49 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/ --- (Updated Aug. 12, 2015, 3:49 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Repository: aurora Description --- Add a new UI page to show all tasks (active and completed) for a specific instance id. N.B.: changes to controllers.js look significant, but it's 90% refactoring so that JobController and JobInstanceController can reuse common code. Diffs - src/main/python/apache/aurora/client/base.py dee4c28a26828912bec073c7e5213a03c9703eee src/main/resources/scheduler/assets/breadcrumb.html 13147935c3c28680d5decd1e03e40d9cf04aaed3 src/main/resources/scheduler/assets/css/app.css a4735efe58a09a46f2b95ebe98f219cb9e7ee27d src/main/resources/scheduler/assets/instance.html PRE-CREATION src/main/resources/scheduler/assets/job.html 942635a61132bb9a38a6faf3f8fe046e6bcabd95 src/main/resources/scheduler/assets/js/app.js b66409f628046c67b62c92a75cbeed20c09b4ec1 src/main/resources/scheduler/assets/js/controllers.js
Re: Review Request 37365: Add a new UI page to show all tasks (active and completed) for a specific instance id.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/#review95107 --- Ship it! Master (cbc42c4) is green with this patch. ./build-support/jenkins/build.sh I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On Aug. 12, 2015, 4:08 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/ --- (Updated Aug. 12, 2015, 4:08 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Repository: aurora Description --- Add a new UI page to show all tasks (active and completed) for a specific instance id. N.B.: changes to controllers.js look significant, but it's 90% refactoring so that JobController and JobInstanceController can reuse common code. Diffs - src/main/python/apache/aurora/client/base.py dee4c28a26828912bec073c7e5213a03c9703eee src/main/resources/scheduler/assets/breadcrumb.html 13147935c3c28680d5decd1e03e40d9cf04aaed3 src/main/resources/scheduler/assets/css/app.css a4735efe58a09a46f2b95ebe98f219cb9e7ee27d src/main/resources/scheduler/assets/instance.html PRE-CREATION src/main/resources/scheduler/assets/job.html 942635a61132bb9a38a6faf3f8fe046e6bcabd95 src/main/resources/scheduler/assets/js/app.js b66409f628046c67b62c92a75cbeed20c09b4ec1 src/main/resources/scheduler/assets/js/controllers.js 04ea1cb6998af7c238eb2fd1a7d05a9663abc56c src/main/resources/scheduler/assets/js/services.js a514fa7da3c049e1f5a3e49263a82cf2c28d1b2d src/main/resources/scheduler/assets/taskInstance.html PRE-CREATION src/test/python/apache/aurora/client/cli/test_supdate.py 21bea702a6e6fe68652e2195c5d24b56b6477738 src/test/python/apache/aurora/client/test_base.py 1a560088279ac945cce14d02454e50b8483771e4 Diff: https://reviews.apache.org/r/37365/diff/ Testing --- - ./gradlew jsHint - ./pants test src/test/python/apache/aurora/client:base - Verified update page url in client was correct in Vagrant. - See attached screenshot for an example of what the instance page looks like. File Attachments Example of instance page in action. https://reviews.apache.org/media/uploaded/files/2015/08/11/2ab72f9d-9e60-4b86-b971-b6aba5465def__Screen_Shot_2015-08-11_at_4.53.07_PM.png Thanks, Joshua Cohen
Re: Review Request 37365: Add a new UI page to show all tasks (active and completed) for a specific instance id.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/ --- (Updated Aug. 12, 2015, 4:08 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Changes --- One last time... appeasing checkstyle this time. Repository: aurora Description --- Add a new UI page to show all tasks (active and completed) for a specific instance id. N.B.: changes to controllers.js look significant, but it's 90% refactoring so that JobController and JobInstanceController can reuse common code. Diffs (updated) - src/main/python/apache/aurora/client/base.py dee4c28a26828912bec073c7e5213a03c9703eee src/main/resources/scheduler/assets/breadcrumb.html 13147935c3c28680d5decd1e03e40d9cf04aaed3 src/main/resources/scheduler/assets/css/app.css a4735efe58a09a46f2b95ebe98f219cb9e7ee27d src/main/resources/scheduler/assets/instance.html PRE-CREATION src/main/resources/scheduler/assets/job.html 942635a61132bb9a38a6faf3f8fe046e6bcabd95 src/main/resources/scheduler/assets/js/app.js b66409f628046c67b62c92a75cbeed20c09b4ec1 src/main/resources/scheduler/assets/js/controllers.js 04ea1cb6998af7c238eb2fd1a7d05a9663abc56c src/main/resources/scheduler/assets/js/services.js a514fa7da3c049e1f5a3e49263a82cf2c28d1b2d src/main/resources/scheduler/assets/taskInstance.html PRE-CREATION src/test/python/apache/aurora/client/cli/test_supdate.py 21bea702a6e6fe68652e2195c5d24b56b6477738 src/test/python/apache/aurora/client/test_base.py 1a560088279ac945cce14d02454e50b8483771e4 Diff: https://reviews.apache.org/r/37365/diff/ Testing --- - ./gradlew jsHint - ./pants test src/test/python/apache/aurora/client:base - Verified update page url in client was correct in Vagrant. - See attached screenshot for an example of what the instance page looks like. File Attachments Example of instance page in action. https://reviews.apache.org/media/uploaded/files/2015/08/11/2ab72f9d-9e60-4b86-b971-b6aba5465def__Screen_Shot_2015-08-11_at_4.53.07_PM.png Thanks, Joshua Cohen
Re: Review Request 37365: Add a new UI page to show all tasks (active and completed) for a specific instance id.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/ --- (Updated Aug. 12, 2015, 4:40 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Changes --- Remove instance_id parameter for the time being. Repository: aurora Description --- Add a new UI page to show all tasks (active and completed) for a specific instance id. N.B.: changes to controllers.js look significant, but it's 90% refactoring so that JobController and JobInstanceController can reuse common code. Diffs (updated) - src/main/python/apache/aurora/client/base.py dee4c28a26828912bec073c7e5213a03c9703eee src/main/resources/scheduler/assets/breadcrumb.html 13147935c3c28680d5decd1e03e40d9cf04aaed3 src/main/resources/scheduler/assets/css/app.css a4735efe58a09a46f2b95ebe98f219cb9e7ee27d src/main/resources/scheduler/assets/instance.html PRE-CREATION src/main/resources/scheduler/assets/job.html 942635a61132bb9a38a6faf3f8fe046e6bcabd95 src/main/resources/scheduler/assets/js/app.js b66409f628046c67b62c92a75cbeed20c09b4ec1 src/main/resources/scheduler/assets/js/controllers.js 04ea1cb6998af7c238eb2fd1a7d05a9663abc56c src/main/resources/scheduler/assets/js/services.js a514fa7da3c049e1f5a3e49263a82cf2c28d1b2d src/main/resources/scheduler/assets/taskInstance.html PRE-CREATION src/test/python/apache/aurora/client/cli/test_supdate.py 21bea702a6e6fe68652e2195c5d24b56b6477738 src/test/python/apache/aurora/client/test_base.py 1a560088279ac945cce14d02454e50b8483771e4 Diff: https://reviews.apache.org/r/37365/diff/ Testing --- - ./gradlew jsHint - ./pants test src/test/python/apache/aurora/client:base - Verified update page url in client was correct in Vagrant. - See attached screenshot for an example of what the instance page looks like. File Attachments Example of instance page in action. https://reviews.apache.org/media/uploaded/files/2015/08/11/2ab72f9d-9e60-4b86-b971-b6aba5465def__Screen_Shot_2015-08-11_at_4.53.07_PM.png Thanks, Joshua Cohen
Re: Review Request 37365: Add a new UI page to show all tasks (active and completed) for a specific instance id.
On Aug. 12, 2015, 4:26 p.m., Maxim Khutornenko wrote: src/main/python/apache/aurora/client/base.py, line 176 https://reviews.apache.org/r/37365/diff/1/?file=1037848#file1037848line176 Drive by comment: Nothing's using this new arg yet. Suggest moving instance_id support in client into a separate diff. Done. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/#review95024 --- On Aug. 12, 2015, 4:40 p.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37365/ --- (Updated Aug. 12, 2015, 4:40 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Repository: aurora Description --- Add a new UI page to show all tasks (active and completed) for a specific instance id. N.B.: changes to controllers.js look significant, but it's 90% refactoring so that JobController and JobInstanceController can reuse common code. Diffs - src/main/python/apache/aurora/client/base.py dee4c28a26828912bec073c7e5213a03c9703eee src/main/resources/scheduler/assets/breadcrumb.html 13147935c3c28680d5decd1e03e40d9cf04aaed3 src/main/resources/scheduler/assets/css/app.css a4735efe58a09a46f2b95ebe98f219cb9e7ee27d src/main/resources/scheduler/assets/instance.html PRE-CREATION src/main/resources/scheduler/assets/job.html 942635a61132bb9a38a6faf3f8fe046e6bcabd95 src/main/resources/scheduler/assets/js/app.js b66409f628046c67b62c92a75cbeed20c09b4ec1 src/main/resources/scheduler/assets/js/controllers.js 04ea1cb6998af7c238eb2fd1a7d05a9663abc56c src/main/resources/scheduler/assets/js/services.js a514fa7da3c049e1f5a3e49263a82cf2c28d1b2d src/main/resources/scheduler/assets/taskInstance.html PRE-CREATION src/test/python/apache/aurora/client/cli/test_supdate.py 21bea702a6e6fe68652e2195c5d24b56b6477738 src/test/python/apache/aurora/client/test_base.py 1a560088279ac945cce14d02454e50b8483771e4 Diff: https://reviews.apache.org/r/37365/diff/ Testing --- - ./gradlew jsHint - ./pants test src/test/python/apache/aurora/client:base - Verified update page url in client was correct in Vagrant. - See attached screenshot for an example of what the instance page looks like. File Attachments Example of instance page in action. https://reviews.apache.org/media/uploaded/files/2015/08/11/2ab72f9d-9e60-4b86-b971-b6aba5465def__Screen_Shot_2015-08-11_at_4.53.07_PM.png Thanks, Joshua Cohen