You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Joshua Cohen <jc...@apache.org> on 2015/08/11 23:56:10 UTC

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/
-----------------------------------------------------------

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/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.


Thanks,

Joshua Cohen


Re: Review Request 37365: Add a new UI page to show all tasks (active and completed) for a specific instance id.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
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 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.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37365/#review95115
-----------------------------------------------------------

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: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
> 
>


Re: Review Request 37365: Add a new UI page to show all tasks (active and completed) for a specific instance id.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
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.

Posted by David McLaughlin <da...@dmclaughlin.com>.
-----------------------------------------------------------
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.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
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.

Posted by Joshua Cohen <jc...@apache.org>.
-----------------------------------------------------------
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.

Posted by Joshua Cohen <jc...@apache.org>.
-----------------------------------------------------------
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.


Changes
-------

Move common job controller functionality to a service.


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.

Posted by Joshua Cohen <jc...@apache.org>.
-----------------------------------------------------------
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.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
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.

Posted by Joshua Cohen <jc...@apache.org>.

> 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.

Posted by Joshua Cohen <jc...@apache.org>.

> 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.

Posted by David McLaughlin <da...@dmclaughlin.com>.

> 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.

So maybe I don't understand why you're using classical inheritance here then, over a vanilla function?


> 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.
> 
> Joshua Cohen wrote:
>     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?

Sounds good. It's a shame angular's default router doesn't allow regexes, as you could have just had a \d+ regex qualifier on instance ids and sent anything else to the update controller for backwards compatibility.


- 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.

Posted by David McLaughlin <da...@dmclaughlin.com>.

> 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.

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?


- David


-----------------------------------------------------------
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.

Posted by David McLaughlin <da...@dmclaughlin.com>.

> 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.

Posted by Joshua Cohen <jc...@apache.org>.

> 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.

Posted by Joshua Cohen <jc...@apache.org>.

> 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.

Posted by David McLaughlin <da...@dmclaughlin.com>.
-----------------------------------------------------------
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.

Posted by Joshua Cohen <jc...@apache.org>.
-----------------------------------------------------------
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.

Posted by Joshua Cohen <jc...@apache.org>.

> 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.

Posted by David McLaughlin <da...@dmclaughlin.com>.
-----------------------------------------------------------
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.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
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.

Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
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.

Posted by Joshua Cohen <jc...@apache.org>.
-----------------------------------------------------------
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.


Changes
-------

- Properly handle the case where there are no active tasks.
- Add a missing `</td>`


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.

Posted by Joshua Cohen <jc...@apache.org>.
-----------------------------------------------------------
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.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
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.

Posted by Joshua Cohen <jc...@apache.org>.

> 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
> 
>


Re: Review Request 37365: Add a new UI page to show all tasks (active and completed) for a specific instance id.

Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37365/#review95024
-----------------------------------------------------------



src/main/python/apache/aurora/client/base.py (line 176)
<https://reviews.apache.org/r/37365/#comment149792>

    Drive by comment: Nothing's using this new arg yet. Suggest moving instance_id support in client into a separate diff.


- Maxim Khutornenko


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.

Posted by Joshua Cohen <jc...@apache.org>.
-----------------------------------------------------------
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.

Posted by Joshua Cohen <jc...@apache.org>.
-----------------------------------------------------------
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.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
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


               FAILURE


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.
> 
> 
> 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/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.

Posted by Joshua Cohen <jc...@apache.org>.
-----------------------------------------------------------
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.


Changes
-------

Appease isort.


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/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.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37365/#review95017
-----------------------------------------------------------


Master (cbc42c4) is red with this patch.
  ./build-support/jenkins/build.sh

virtualenv-12.1.1/virtualenv_embedded/
virtualenv-12.1.1/virtualenv_embedded/activate.bat
virtualenv-12.1.1/virtualenv_embedded/activate.csh
virtualenv-12.1.1/virtualenv_embedded/activate.fish
virtualenv-12.1.1/virtualenv_embedded/activate.ps1
virtualenv-12.1.1/virtualenv_embedded/activate.sh
virtualenv-12.1.1/virtualenv_embedded/activate_this.py
virtualenv-12.1.1/virtualenv_embedded/deactivate.bat
virtualenv-12.1.1/virtualenv_embedded/distutils-init.py
virtualenv-12.1.1/virtualenv_embedded/distutils.cfg
virtualenv-12.1.1/virtualenv_embedded/site.py
virtualenv-12.1.1/virtualenv_support/
virtualenv-12.1.1/virtualenv_support/__init__.py
virtualenv-12.1.1/virtualenv_support/pip-6.1.1-py2.py3-none-any.whl
virtualenv-12.1.1/virtualenv_support/setuptools-15.0-py2.py3-none-any.whl
+ touch virtualenv-12.1.1/BOOTSTRAPPED
+ popd
~/jenkins-slave/workspace/AuroraBot
+ exec /usr/bin/python2.7 /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-12.1.1/virtualenv.py /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/isort.venv
New python executable in /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/isort.venv/bin/python2.7
Also creating executable in /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/isort.venv/bin/python
Installing setuptools, pip...done.
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/isort.venv/local/lib/python2.7/site-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:79: InsecurePlatformWarning: A true SSLContext object is not available. This prevents urllib3 from configuring SSL appropriately and may cause certain SSL connections to fail. For more information, see https://urllib3.readthedocs.org/en/latest/security.html#insecureplatformwarning.
  InsecurePlatformWarning
You are using pip version 6.1.1, however version 7.1.0 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.
Collecting isort==4.0.0
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/isort.venv/local/lib/python2.7/site-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:79: InsecurePlatformWarning: A true SSLContext object is not available. This prevents urllib3 from configuring SSL appropriately and may cause certain SSL connections to fail. For more information, see https://urllib3.readthedocs.org/en/latest/security.html#insecureplatformwarning.
  InsecurePlatformWarning
  Using cached isort-4.0.0-py2.py3-none-any.whl
Installing collected packages: isort
Successfully installed isort-4.0.0
ERROR: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/test_base.py Imports are incorrectly sorted.
--- /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/test_base.py:before	2015-08-11 22:11:43.548073
+++ /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/python/apache/aurora/client/test_base.py:after	2015-08-11 22:18:11.190114
@@ -82,4 +82,3 @@
 
     assert (('%s/scheduler/%s' % (base_url, role)) ==
         base.synthesize_url(base_url, role))
-


I will refresh this build result if you post a review containing "@ReviewBot retry"

- Aurora ReviewBot


On Aug. 11, 2015, 9:56 p.m., Joshua Cohen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37365/
> -----------------------------------------------------------
> 
> (Updated Aug. 11, 2015, 9:56 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/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.

Posted by Joshua Cohen <jc...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37365/
-----------------------------------------------------------

(Updated Aug. 11, 2015, 9:56 p.m.)


Review request for Aurora, David McLaughlin and Maxim Khutornenko.


Changes
-------

Add screenshot.


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/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 (updated)
----------------

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