Re: Review Request 63337: Polling updates page if in progress in UI

2017-10-27 Thread Reza Motamedi

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63337/
---

(Updated Oct. 27, 2017, 5:01 p.m.)


Review request for Aurora and David McLaughlin.


Changes
---

Change my approach slightly in the version. Instead on using `setInterval`, I 
use `setTimeout` which is more appropriate here! 
- Moved logic around and addressed review feedback.
- Also added tests.


Repository: aurora


Description
---

# Polling updates page if in progress in UI

This will update the job update page every 5 seconds if the update is in 
progress. It was also stop refreshing the page if the update is not in progress 
anymore.


Diffs (updated)
-

  ui/src/main/js/pages/Update.js c900269f0e47ccde616f021a7ecd51e13c57b734 
  ui/src/main/js/pages/__tests__/Update-test.js 
570a999ccd60c646a1206941e968714e5457b1a4 


Diff: https://reviews.apache.org/r/63337/diff/3/

Changes: https://reviews.apache.org/r/63337/diff/2-3/


Testing
---

Checked in vagrant with a tunneled live scheduler and it works. Current tests 
pass.


```
# rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:ui-update-polling ? [13:31:26]
? ./gradlew ui:lint

> Task :ui:lint

> apache-aurora@1.0.0 lint /Users/rmotamedi/oss/aurora/ui
> eslint src/main/js plugin/ --ext .js


BUILD SUCCESSFUL in 15s
4 actionable tasks: 1 executed, 3 up-to-date

# rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:ui-update-polling ? [13:31:45]
? ./gradlew ui:test

> Task :ui:test

> apache-aurora@1.0.0 test /Users/rmotamedi/oss/aurora/ui
> jest src/ plugin/

PASS src/main/js/pages/__tests__/Job-test.js
PASS src/main/js/components/__tests__/InstanceViz-test.js
PASS src/main/js/components/__tests__/ConfigDiff-test.js
PASS src/main/js/components/__tests__/Diff-test.js
PASS src/main/js/components/__tests__/InstanceHistoryItem-test.js
PASS src/main/js/utils/__tests__/Update-test.js
PASS src/main/js/components/__tests__/Pagination-test.js
PASS src/main/js/pages/__tests__/Update-test.js
PASS src/main/js/components/__tests__/UpdateList-test.js
PASS src/main/js/components/__tests__/UpdateInstanceEvents-test.js
PASS src/main/js/components/__tests__/JobStatus-test.js
PASS src/main/js/pages/__tests__/Instance-test.js
PASS src/main/js/components/__tests__/UpdateStatus-test.js
PASS src/main/js/components/__tests__/UpdateDiff-test.js
PASS src/main/js/components/__tests__/JobHistory-test.js
PASS src/main/js/components/__tests__/TaskNeighbors-test.js
PASS src/main/js/pages/__tests__/Updates-test.js
PASS src/main/js/pages/__tests__/Home-test.js
PASS src/main/js/components/__tests__/JobConfig-test.js
PASS src/main/js/test-utils/__tests__/Builder-test.js
PASS src/main/js/components/__tests__/JobList-test.js
PASS src/main/js/components/__tests__/Breadcrumb-test.js
PASS src/main/js/utils/__tests__/Task-test.js
PASS src/main/js/components/__tests__/Tabs-test.js
PASS src/main/js/components/__tests__/InstanceHistory-test.js
PASS src/main/js/components/__tests__/TaskList-test.js
PASS src/main/js/pages/__tests__/Jobs-test.js
PASS src/main/js/components/__tests__/RoleQuota-test.js
PASS src/main/js/utils/__tests__/Common-test.js
PASS src/main/js/components/__tests__/StateMachine-test.js

Test Suites: 30 passed, 30 total
Tests:   134 passed, 134 total
Snapshots:   0 total
Time:6.118s
Ran all test suites matching /src\/|plugin\//i.


BUILD SUCCESSFUL in 9s
2 actionable tasks: 1 executed, 1 up-to-date

```


Thanks,

Reza Motamedi



Re: Review Request 63364: Revert role searching in UI to old behavior

2017-10-27 Thread Reza Motamedi

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63364/#review189442
---


Ship it!




Ship It!

- Reza Motamedi


On Oct. 27, 2017, 4:42 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63364/
> ---
> 
> (Updated Oct. 27, 2017, 4:42 p.m.)
> 
> 
> Review request for Aurora, Kai Huang and Reza Motamedi.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Move from prefix search to full-text matching.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/RoleList.js 
> 2bdae8e0f5f3a066a5a4997c0034b1ff2a5ff8cf 
> 
> 
> Diff: https://reviews.apache.org/r/63364/diff/1/
> 
> 
> Testing
> ---
> 
> Tested in Vagrant.
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>



Re: Review Request 63364: Revert role searching in UI to old behavior

2017-10-27 Thread Aurora ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63364/#review189446
---


Ship it!




Master (5a26c8b) 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 Oct. 27, 2017, 4:42 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63364/
> ---
> 
> (Updated Oct. 27, 2017, 4:42 p.m.)
> 
> 
> Review request for Aurora, Kai Huang and Reza Motamedi.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Move from prefix search to full-text matching.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/RoleList.js 
> 2bdae8e0f5f3a066a5a4997c0034b1ff2a5ff8cf 
> 
> 
> Diff: https://reviews.apache.org/r/63364/diff/1/
> 
> 
> Testing
> ---
> 
> Tested in Vagrant.
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>



Re: Review Request 63337: Polling updates page if in progress in UI

2017-10-27 Thread Aurora ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63337/#review189447
---



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

thrift-0.9.1/tutorial/py/PythonClient.py
thrift-0.9.1/tutorial/py/PythonServer.py
thrift-0.9.1/tutorial/py.tornado/
thrift-0.9.1/tutorial/py.tornado/Makefile.am
thrift-0.9.1/tutorial/py.tornado/Makefile.in
thrift-0.9.1/tutorial/py.tornado/PythonClient.py
thrift-0.9.1/tutorial/py.tornado/PythonServer.py
thrift-0.9.1/tutorial/py.twisted/
thrift-0.9.1/tutorial/py.twisted/Makefile.am
thrift-0.9.1/tutorial/py.twisted/Makefile.in
thrift-0.9.1/tutorial/py.twisted/PythonClient.py
thrift-0.9.1/tutorial/py.twisted/PythonServer.py
thrift-0.9.1/tutorial/py.twisted/PythonServer.tac
thrift-0.9.1/tutorial/rb/
thrift-0.9.1/tutorial/rb/Makefile.am
thrift-0.9.1/tutorial/rb/Makefile.in
thrift-0.9.1/tutorial/rb/RubyClient.rb
thrift-0.9.1/tutorial/rb/RubyServer.rb
thrift-0.9.1/tutorial/README
thrift-0.9.1/tutorial/shared.thrift
thrift-0.9.1/tutorial/tutorial.thrift
thrift-0.9.1/ylwrap
Makefile:49: recipe for target 'thrift-0.9.1/compiler/cpp/thrift' failed
make: *** [thrift-0.9.1/compiler/cpp/thrift] Error 1
make: Leaving directory 
'/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift'
:api:generateThriftJava FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':api:generateThriftJava'.
> Process 'command 
> '/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift/thriftw''
>  finished with non-zero exit value 2

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.

* Get more help at https://help.gradle.org

BUILD FAILED in 24s
7 actionable tasks: 1 executed, 6 up-to-date


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

- Aurora ReviewBot


On Oct. 27, 2017, 5:01 p.m., Reza Motamedi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63337/
> ---
> 
> (Updated Oct. 27, 2017, 5:01 p.m.)
> 
> 
> Review request for Aurora and David McLaughlin.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> # Polling updates page if in progress in UI
> 
> This will update the job update page every 5 seconds if the update is in 
> progress. It was also stop refreshing the page if the update is not in 
> progress anymore.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/pages/Update.js c900269f0e47ccde616f021a7ecd51e13c57b734 
>   ui/src/main/js/pages/__tests__/Update-test.js 
> 570a999ccd60c646a1206941e968714e5457b1a4 
> 
> 
> Diff: https://reviews.apache.org/r/63337/diff/3/
> 
> 
> Testing
> ---
> 
> Checked in vagrant with a tunneled live scheduler and it works. Current tests 
> pass.
> 
> 
> ```
> # rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:ui-update-polling ? 
> [13:31:26]
> ? ./gradlew ui:lint
> 
> > Task :ui:lint
> 
> > apache-aurora@1.0.0 lint /Users/rmotamedi/oss/aurora/ui
> > eslint src/main/js plugin/ --ext .js
> 
> 
> BUILD SUCCESSFUL in 15s
> 4 actionable tasks: 1 executed, 3 up-to-date
> 
> # rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:ui-update-polling ? 
> [13:31:45]
> ? ./gradlew ui:test
> 
> > Task :ui:test
> 
> > apache-aurora@1.0.0 test /Users/rmotamedi/oss/aurora/ui
> > jest src/ plugin/
> 
> PASS src/main/js/pages/__tests__/Job-test.js
> PASS src/main/js/components/__tests__/InstanceViz-test.js
> PASS src/main/js/components/__tests__/ConfigDiff-test.js
> PASS src/main/js/components/__tests__/Diff-test.js
> PASS src/main/js/components/__tests__/InstanceHistoryItem-test.js
> PASS src/main/js/utils/__tests__/Update-test.js
> PASS src/main/js/components/__tests__/Pagination-test.js
> PASS src/main/js/pages/__tests__/Update-test.js
> PASS src/main/js/components/__tests__/UpdateList-test.js
> PASS src/main/js/components/__tests__/UpdateInstanceEvents-test.js
> PASS src/main/js/components/__tests__/JobStatus-test.js
> PASS src/main/js/pages/__tests__/Instance-test.js
> PASS src/main/js/components/__tests__/UpdateStatus-test.js
> PASS src/main/js/components/__tests__/UpdateDiff-test.js
> PASS src/main/js/components/__tests__/JobHistory-test.js
> PASS src/main/js/components/__tests__/TaskNeighbors-test.js
> PASS src/main/js/pages/__tests__/Updates-test.js
> PASS src/main/js/pages/__tests__/Home-test.js
> PASS src/main/js/components/__tests__/JobConfig-test.js
> PASS src/main/js/test-utils/__tests__/Builder-test.js
> PASS src/main/js/components/__tests__/JobList-test.js
> PASS src/main/js/components/__tests__/Breadcrumb-test.js
> PASS src/main/js/utils/__tests__/Task-test.js
> PASS src/main/js/components/__tests__/Tabs-test.js
> PASS 

Review Request 63364: Revert role searching in UI to old behavior

2017-10-27 Thread David McLaughlin

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63364/
---

Review request for Aurora, Kai Huang and Reza Motamedi.


Repository: aurora


Description
---

Move from prefix search to full-text matching.


Diffs
-

  ui/src/main/js/components/RoleList.js 
2bdae8e0f5f3a066a5a4997c0034b1ff2a5ff8cf 


Diff: https://reviews.apache.org/r/63364/diff/1/


Testing
---

Tested in Vagrant.


Thanks,

David McLaughlin



Re: Review Request 63337: Polling updates page if in progress in UI

2017-10-27 Thread Reza Motamedi

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63337/#review189465
---



@ReviewBot retry

- Reza Motamedi


On Oct. 27, 2017, 5:01 p.m., Reza Motamedi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63337/
> ---
> 
> (Updated Oct. 27, 2017, 5:01 p.m.)
> 
> 
> Review request for Aurora and David McLaughlin.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> # Polling updates page if in progress in UI
> 
> This will update the job update page every 5 seconds if the update is in 
> progress. It was also stop refreshing the page if the update is not in 
> progress anymore.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/pages/Update.js c900269f0e47ccde616f021a7ecd51e13c57b734 
>   ui/src/main/js/pages/__tests__/Update-test.js 
> 570a999ccd60c646a1206941e968714e5457b1a4 
> 
> 
> Diff: https://reviews.apache.org/r/63337/diff/3/
> 
> 
> Testing
> ---
> 
> Checked in vagrant with a tunneled live scheduler and it works. Current tests 
> pass.
> 
> 
> ```
> # rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:ui-update-polling ? 
> [13:31:26]
> ? ./gradlew ui:lint
> 
> > Task :ui:lint
> 
> > apache-aurora@1.0.0 lint /Users/rmotamedi/oss/aurora/ui
> > eslint src/main/js plugin/ --ext .js
> 
> 
> BUILD SUCCESSFUL in 15s
> 4 actionable tasks: 1 executed, 3 up-to-date
> 
> # rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:ui-update-polling ? 
> [13:31:45]
> ? ./gradlew ui:test
> 
> > Task :ui:test
> 
> > apache-aurora@1.0.0 test /Users/rmotamedi/oss/aurora/ui
> > jest src/ plugin/
> 
> PASS src/main/js/pages/__tests__/Job-test.js
> PASS src/main/js/components/__tests__/InstanceViz-test.js
> PASS src/main/js/components/__tests__/ConfigDiff-test.js
> PASS src/main/js/components/__tests__/Diff-test.js
> PASS src/main/js/components/__tests__/InstanceHistoryItem-test.js
> PASS src/main/js/utils/__tests__/Update-test.js
> PASS src/main/js/components/__tests__/Pagination-test.js
> PASS src/main/js/pages/__tests__/Update-test.js
> PASS src/main/js/components/__tests__/UpdateList-test.js
> PASS src/main/js/components/__tests__/UpdateInstanceEvents-test.js
> PASS src/main/js/components/__tests__/JobStatus-test.js
> PASS src/main/js/pages/__tests__/Instance-test.js
> PASS src/main/js/components/__tests__/UpdateStatus-test.js
> PASS src/main/js/components/__tests__/UpdateDiff-test.js
> PASS src/main/js/components/__tests__/JobHistory-test.js
> PASS src/main/js/components/__tests__/TaskNeighbors-test.js
> PASS src/main/js/pages/__tests__/Updates-test.js
> PASS src/main/js/pages/__tests__/Home-test.js
> PASS src/main/js/components/__tests__/JobConfig-test.js
> PASS src/main/js/test-utils/__tests__/Builder-test.js
> PASS src/main/js/components/__tests__/JobList-test.js
> PASS src/main/js/components/__tests__/Breadcrumb-test.js
> PASS src/main/js/utils/__tests__/Task-test.js
> PASS src/main/js/components/__tests__/Tabs-test.js
> PASS src/main/js/components/__tests__/InstanceHistory-test.js
> PASS src/main/js/components/__tests__/TaskList-test.js
> PASS src/main/js/pages/__tests__/Jobs-test.js
> PASS src/main/js/components/__tests__/RoleQuota-test.js
> PASS src/main/js/utils/__tests__/Common-test.js
> PASS src/main/js/components/__tests__/StateMachine-test.js
> 
> Test Suites: 30 passed, 30 total
> Tests:   134 passed, 134 total
> Snapshots:   0 total
> Time:6.118s
> Ran all test suites matching /src\/|plugin\//i.
> 
> 
> BUILD SUCCESSFUL in 9s
> 2 actionable tasks: 1 executed, 1 up-to-date
> 
> ```
> 
> 
> Thanks,
> 
> Reza Motamedi
> 
>



Re: Review Request 63337: Polling updates page if in progress in UI

2017-10-27 Thread Aurora ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63337/#review189466
---



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

thrift-0.9.1/tutorial/py/Makefile.in
thrift-0.9.1/tutorial/py/PythonClient.py
thrift-0.9.1/tutorial/py/PythonServer.py
thrift-0.9.1/tutorial/py.tornado/
thrift-0.9.1/tutorial/py.tornado/Makefile.am
thrift-0.9.1/tutorial/py.tornado/Makefile.in
thrift-0.9.1/tutorial/py.tornado/PythonClient.py
thrift-0.9.1/tutorial/py.tornado/PythonServer.py
thrift-0.9.1/tutorial/py.twisted/
thrift-0.9.1/tutorial/py.twisted/Makefile.am
thrift-0.9.1/tutorial/py.twisted/Makefile.in
thrift-0.9.1/tutorial/py.twisted/PythonClient.py
thrift-0.9.1/tutorial/py.twisted/PythonServer.py
thrift-0.9.1/tutorial/py.twisted/PythonServer.tac
thrift-0.9.1/tutorial/rb/
thrift-0.9.1/tutorial/rb/Makefile.am
thrift-0.9.1/tutorial/rb/Makefile.in
thrift-0.9.1/tutorial/rb/RubyClient.rb
thrift-0.9.1/tutorial/rb/RubyServer.rb
thrift-0.9.1/tutorial/README
thrift-0.9.1/tutorial/shared.thrift
thrift-0.9.1/tutorial/tutorial.thrift
thrift-0.9.1/ylwrap
make: *** [thrift-0.9.1/compiler/cpp/thrift] Error 1
make: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift'
:api:generateThriftJava FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':api:generateThriftJava'.
> Process 'command 
> '/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift/thriftw''
>  finished with non-zero exit value 2

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.

* Get more help at https://help.gradle.org

BUILD FAILED in 25s
7 actionable tasks: 1 executed, 6 up-to-date


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

- Aurora ReviewBot


On Oct. 27, 2017, 5:01 p.m., Reza Motamedi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63337/
> ---
> 
> (Updated Oct. 27, 2017, 5:01 p.m.)
> 
> 
> Review request for Aurora and David McLaughlin.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> # Polling updates page if in progress in UI
> 
> This will update the job update page every 5 seconds if the update is in 
> progress. It was also stop refreshing the page if the update is not in 
> progress anymore.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/pages/Update.js c900269f0e47ccde616f021a7ecd51e13c57b734 
>   ui/src/main/js/pages/__tests__/Update-test.js 
> 570a999ccd60c646a1206941e968714e5457b1a4 
> 
> 
> Diff: https://reviews.apache.org/r/63337/diff/3/
> 
> 
> Testing
> ---
> 
> Checked in vagrant with a tunneled live scheduler and it works. Current tests 
> pass.
> 
> 
> ```
> # rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:ui-update-polling ? 
> [13:31:26]
> ? ./gradlew ui:lint
> 
> > Task :ui:lint
> 
> > apache-aurora@1.0.0 lint /Users/rmotamedi/oss/aurora/ui
> > eslint src/main/js plugin/ --ext .js
> 
> 
> BUILD SUCCESSFUL in 15s
> 4 actionable tasks: 1 executed, 3 up-to-date
> 
> # rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:ui-update-polling ? 
> [13:31:45]
> ? ./gradlew ui:test
> 
> > Task :ui:test
> 
> > apache-aurora@1.0.0 test /Users/rmotamedi/oss/aurora/ui
> > jest src/ plugin/
> 
> PASS src/main/js/pages/__tests__/Job-test.js
> PASS src/main/js/components/__tests__/InstanceViz-test.js
> PASS src/main/js/components/__tests__/ConfigDiff-test.js
> PASS src/main/js/components/__tests__/Diff-test.js
> PASS src/main/js/components/__tests__/InstanceHistoryItem-test.js
> PASS src/main/js/utils/__tests__/Update-test.js
> PASS src/main/js/components/__tests__/Pagination-test.js
> PASS src/main/js/pages/__tests__/Update-test.js
> PASS src/main/js/components/__tests__/UpdateList-test.js
> PASS src/main/js/components/__tests__/UpdateInstanceEvents-test.js
> PASS src/main/js/components/__tests__/JobStatus-test.js
> PASS src/main/js/pages/__tests__/Instance-test.js
> PASS src/main/js/components/__tests__/UpdateStatus-test.js
> PASS src/main/js/components/__tests__/UpdateDiff-test.js
> PASS src/main/js/components/__tests__/JobHistory-test.js
> PASS src/main/js/components/__tests__/TaskNeighbors-test.js
> PASS src/main/js/pages/__tests__/Updates-test.js
> PASS src/main/js/pages/__tests__/Home-test.js
> PASS src/main/js/components/__tests__/JobConfig-test.js
> PASS src/main/js/test-utils/__tests__/Builder-test.js
> PASS src/main/js/components/__tests__/JobList-test.js
> PASS src/main/js/components/__tests__/Breadcrumb-test.js
> PASS src/main/js/utils/__tests__/Task-test.js
> PASS src/main/js/components/__tests__/Tabs-test.js
> PASS src/main/js/components/__tests__/InstanceHistory-test.js
> PASS 

Review Request 63373: Reduce white-space on role and env pages

2017-10-27 Thread David McLaughlin

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63373/
---

Review request for Aurora, Kai Huang, Santhosh Kumar Shanmugham, and Stephan 
Erb.


Repository: aurora


Description
---

We had a request to show more content on the job list (the new UI increases the 
size of an item by about 20px per item).


Diffs
-

  ui/src/main/sass/components/_job-list-page.scss 
a716b9869b8a00d8232eacdd79c47f4b63055217 


Diff: https://reviews.apache.org/r/63373/diff/1/


Testing
---

See before and after below.


File Attachments


Reduced
  
https://reviews.apache.org/media/uploaded/files/2017/10/27/e09fae56-a110-4e68-a7c4-6317f366dc58__Screen_Shot_2017-10-27_at_12.31.38_PM.png
Before
  
https://reviews.apache.org/media/uploaded/files/2017/10/27/6242deee-9916-4df2-8c34-7fed89ec1b94__Screen_Shot_2017-10-27_at_12.31.46_PM.png


Thanks,

David McLaughlin



Review Request 63374: Revert to old Job Page tab names and add counts

2017-10-27 Thread David McLaughlin

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63374/
---

Review request for Aurora, Kai Huang and Reza Motamedi.


Repository: aurora


Description
---

Changing the names of tabs causes unnecessary confusion. Revert to "active 
tasks/completed tasks" and add the instance count back to both.


Diffs
-

  ui/src/main/js/components/JobHistory.js 
74f6fcb1117e80bcd5a033dad06ca00f9bc2ddfc 
  ui/src/main/js/components/JobStatus.js 
84e335c91da7231eda30ae945dd669f80fb1cc38 


Diff: https://reviews.apache.org/r/63374/diff/1/


Testing
---

See screenshots.


File Attachments


Screen Shot 2017-10-27 at 12.46.52 PM.png
  
https://reviews.apache.org/media/uploaded/files/2017/10/27/33061824-a52c-4b54-a14f-27793f5bc9ea__Screen_Shot_2017-10-27_at_12.46.52_PM.png


Thanks,

David McLaughlin



Re: Review Request 63374: Revert to old Job Page tab names and add counts

2017-10-27 Thread Aurora ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63374/#review189469
---



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

thrift-0.9.1/tutorial/py/PythonClient.py
thrift-0.9.1/tutorial/py/PythonServer.py
thrift-0.9.1/tutorial/py.tornado/
thrift-0.9.1/tutorial/py.tornado/Makefile.am
thrift-0.9.1/tutorial/py.tornado/Makefile.in
thrift-0.9.1/tutorial/py.tornado/PythonClient.py
thrift-0.9.1/tutorial/py.tornado/PythonServer.py
thrift-0.9.1/tutorial/py.twisted/
thrift-0.9.1/tutorial/py.twisted/Makefile.am
thrift-0.9.1/tutorial/py.twisted/Makefile.in
thrift-0.9.1/tutorial/py.twisted/PythonClient.py
thrift-0.9.1/tutorial/py.twisted/PythonServer.py
thrift-0.9.1/tutorial/py.twisted/PythonServer.tac
thrift-0.9.1/tutorial/rb/
thrift-0.9.1/tutorial/rb/Makefile.am
thrift-0.9.1/tutorial/rb/Makefile.in
thrift-0.9.1/tutorial/rb/RubyClient.rb
thrift-0.9.1/tutorial/rb/RubyServer.rb
thrift-0.9.1/tutorial/README
thrift-0.9.1/tutorial/shared.thrift
thrift-0.9.1/tutorial/tutorial.thrift
thrift-0.9.1/ylwrap
Makefile:49: recipe for target 'thrift-0.9.1/compiler/cpp/thrift' failed
make: Leaving directory 
'/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift'
make: *** [thrift-0.9.1/compiler/cpp/thrift] Error 1
:api:generateThriftJava FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':api:generateThriftJava'.
> Process 'command 
> '/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift/thriftw''
>  finished with non-zero exit value 2

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.

* Get more help at https://help.gradle.org

BUILD FAILED in 14s
7 actionable tasks: 1 executed, 6 up-to-date


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

- Aurora ReviewBot


On Oct. 27, 2017, 7:48 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63374/
> ---
> 
> (Updated Oct. 27, 2017, 7:48 p.m.)
> 
> 
> Review request for Aurora, Kai Huang and Reza Motamedi.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Changing the names of tabs causes unnecessary confusion. Revert to "active 
> tasks/completed tasks" and add the instance count back to both.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/JobHistory.js 
> 74f6fcb1117e80bcd5a033dad06ca00f9bc2ddfc 
>   ui/src/main/js/components/JobStatus.js 
> 84e335c91da7231eda30ae945dd669f80fb1cc38 
> 
> 
> Diff: https://reviews.apache.org/r/63374/diff/1/
> 
> 
> Testing
> ---
> 
> See screenshots.
> 
> 
> File Attachments
> 
> 
> Screen Shot 2017-10-27 at 12.46.52 PM.png
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/27/33061824-a52c-4b54-a14f-27793f5bc9ea__Screen_Shot_2017-10-27_at_12.46.52_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>



Re: Review Request 63373: Reduce white-space on role and env pages

2017-10-27 Thread Aurora ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63373/#review189470
---



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

thrift-0.9.1/tutorial/py/PythonClient.py
thrift-0.9.1/tutorial/py/PythonServer.py
thrift-0.9.1/tutorial/py.tornado/
thrift-0.9.1/tutorial/py.tornado/Makefile.am
thrift-0.9.1/tutorial/py.tornado/Makefile.in
thrift-0.9.1/tutorial/py.tornado/PythonClient.py
thrift-0.9.1/tutorial/py.tornado/PythonServer.py
thrift-0.9.1/tutorial/py.twisted/
thrift-0.9.1/tutorial/py.twisted/Makefile.am
thrift-0.9.1/tutorial/py.twisted/Makefile.in
thrift-0.9.1/tutorial/py.twisted/PythonClient.py
thrift-0.9.1/tutorial/py.twisted/PythonServer.py
thrift-0.9.1/tutorial/py.twisted/PythonServer.tac
thrift-0.9.1/tutorial/rb/
thrift-0.9.1/tutorial/rb/Makefile.am
thrift-0.9.1/tutorial/rb/Makefile.in
thrift-0.9.1/tutorial/rb/RubyClient.rb
thrift-0.9.1/tutorial/rb/RubyServer.rb
thrift-0.9.1/tutorial/README
thrift-0.9.1/tutorial/shared.thrift
thrift-0.9.1/tutorial/tutorial.thrift
thrift-0.9.1/ylwrap
Makefile:49: recipe for target 'thrift-0.9.1/compiler/cpp/thrift' failed
make: Leaving directory 
'/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift'
make: *** [thrift-0.9.1/compiler/cpp/thrift] Error 1
:api:generateThriftJava FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':api:generateThriftJava'.
> Process 'command 
> '/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift/thriftw''
>  finished with non-zero exit value 2

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.

* Get more help at https://help.gradle.org

BUILD FAILED in 15s
7 actionable tasks: 1 executed, 6 up-to-date


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

- Aurora ReviewBot


On Oct. 27, 2017, 7:35 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63373/
> ---
> 
> (Updated Oct. 27, 2017, 7:35 p.m.)
> 
> 
> Review request for Aurora, Kai Huang, Santhosh Kumar Shanmugham, and Stephan 
> Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> We had a request to show more content on the job list (the new UI increases 
> the size of an item by about 20px per item).
> 
> 
> Diffs
> -
> 
>   ui/src/main/sass/components/_job-list-page.scss 
> a716b9869b8a00d8232eacdd79c47f4b63055217 
> 
> 
> Diff: https://reviews.apache.org/r/63373/diff/1/
> 
> 
> Testing
> ---
> 
> See before and after below.
> 
> 
> File Attachments
> 
> 
> Reduced
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/27/e09fae56-a110-4e68-a7c4-6317f366dc58__Screen_Shot_2017-10-27_at_12.31.38_PM.png
> Before
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/27/6242deee-9916-4df2-8c34-7fed89ec1b94__Screen_Shot_2017-10-27_at_12.31.46_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>



Re: Review Request 63374: Revert to old Job Page tab names and add counts

2017-10-27 Thread Reza Motamedi

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63374/#review189472
---


Ship it!




Ship It!

- Reza Motamedi


On Oct. 27, 2017, 7:48 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63374/
> ---
> 
> (Updated Oct. 27, 2017, 7:48 p.m.)
> 
> 
> Review request for Aurora, Kai Huang and Reza Motamedi.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Changing the names of tabs causes unnecessary confusion. Revert to "active 
> tasks/completed tasks" and add the instance count back to both.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/JobHistory.js 
> 74f6fcb1117e80bcd5a033dad06ca00f9bc2ddfc 
>   ui/src/main/js/components/JobStatus.js 
> 84e335c91da7231eda30ae945dd669f80fb1cc38 
> 
> 
> Diff: https://reviews.apache.org/r/63374/diff/1/
> 
> 
> Testing
> ---
> 
> See screenshots.
> 
> 
> File Attachments
> 
> 
> Screen Shot 2017-10-27 at 12.46.52 PM.png
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/27/33061824-a52c-4b54-a14f-27793f5bc9ea__Screen_Shot_2017-10-27_at_12.46.52_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>



Review Request 63375: Add resource units to config summary

2017-10-27 Thread David McLaughlin

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63375/
---

Review request for Aurora, Kai Huang and Reza Motamedi.


Repository: aurora


Description
---

Add resource units to config summary


Diffs
-

  ui/src/main/js/components/RoleQuota.js 
bb1b6e7e5dbedf875f27caf9bc9ca28319b82c5b 
  ui/src/main/js/components/TaskConfigSummary.js 
b1cf5a956e0230932944f76ad8d25b356b9d8f23 
  ui/src/main/js/utils/Quota.js PRE-CREATION 


Diff: https://reviews.apache.org/r/63375/diff/1/


Testing
---

See screenshot.


File Attachments


Screen Shot 2017-10-27 at 12.55.52 PM.png
  
https://reviews.apache.org/media/uploaded/files/2017/10/27/be64586c-3ab8-4e17-bec2-b166719da2e7__Screen_Shot_2017-10-27_at_12.55.52_PM.png


Thanks,

David McLaughlin



Re: Review Request 63375: Add resource units to config summary

2017-10-27 Thread Aurora ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63375/#review189473
---



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

thrift-0.9.1/tutorial/py/PythonClient.py
thrift-0.9.1/tutorial/py/PythonServer.py
thrift-0.9.1/tutorial/py.tornado/
thrift-0.9.1/tutorial/py.tornado/Makefile.am
thrift-0.9.1/tutorial/py.tornado/Makefile.in
thrift-0.9.1/tutorial/py.tornado/PythonClient.py
thrift-0.9.1/tutorial/py.tornado/PythonServer.py
thrift-0.9.1/tutorial/py.twisted/
thrift-0.9.1/tutorial/py.twisted/Makefile.am
thrift-0.9.1/tutorial/py.twisted/Makefile.in
thrift-0.9.1/tutorial/py.twisted/PythonClient.py
thrift-0.9.1/tutorial/py.twisted/PythonServer.py
thrift-0.9.1/tutorial/py.twisted/PythonServer.tac
thrift-0.9.1/tutorial/rb/
thrift-0.9.1/tutorial/rb/Makefile.am
thrift-0.9.1/tutorial/rb/Makefile.in
thrift-0.9.1/tutorial/rb/RubyClient.rb
thrift-0.9.1/tutorial/rb/RubyServer.rb
thrift-0.9.1/tutorial/README
thrift-0.9.1/tutorial/shared.thrift
thrift-0.9.1/tutorial/tutorial.thrift
thrift-0.9.1/ylwrap
Makefile:49: recipe for target 'thrift-0.9.1/compiler/cpp/thrift' failed
make: Leaving directory 
'/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift'
make: *** [thrift-0.9.1/compiler/cpp/thrift] Error 1
:api:generateThriftJava FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':api:generateThriftJava'.
> Process 'command 
> '/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift/thriftw''
>  finished with non-zero exit value 2

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.

* Get more help at https://help.gradle.org

BUILD FAILED in 39s
7 actionable tasks: 1 executed, 6 up-to-date


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

- Aurora ReviewBot


On Oct. 27, 2017, 7:58 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63375/
> ---
> 
> (Updated Oct. 27, 2017, 7:58 p.m.)
> 
> 
> Review request for Aurora, Kai Huang and Reza Motamedi.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add resource units to config summary
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/RoleQuota.js 
> bb1b6e7e5dbedf875f27caf9bc9ca28319b82c5b 
>   ui/src/main/js/components/TaskConfigSummary.js 
> b1cf5a956e0230932944f76ad8d25b356b9d8f23 
>   ui/src/main/js/utils/Quota.js PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63375/diff/1/
> 
> 
> Testing
> ---
> 
> See screenshot.
> 
> 
> File Attachments
> 
> 
> Screen Shot 2017-10-27 at 12.55.52 PM.png
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/27/be64586c-3ab8-4e17-bec2-b166719da2e7__Screen_Shot_2017-10-27_at_12.55.52_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>



Re: Review Request 63375: Add resource units to config summary

2017-10-27 Thread Kai Huang

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63375/#review189480
---


Ship it!




Ship It!

- Kai Huang


On Oct. 27, 2017, 7:58 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63375/
> ---
> 
> (Updated Oct. 27, 2017, 7:58 p.m.)
> 
> 
> Review request for Aurora, Kai Huang and Reza Motamedi.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add resource units to config summary
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/RoleQuota.js 
> bb1b6e7e5dbedf875f27caf9bc9ca28319b82c5b 
>   ui/src/main/js/components/TaskConfigSummary.js 
> b1cf5a956e0230932944f76ad8d25b356b9d8f23 
>   ui/src/main/js/utils/Quota.js PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63375/diff/1/
> 
> 
> Testing
> ---
> 
> See screenshot.
> 
> 
> File Attachments
> 
> 
> Screen Shot 2017-10-27 at 12.55.52 PM.png
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/27/be64586c-3ab8-4e17-bec2-b166719da2e7__Screen_Shot_2017-10-27_at_12.55.52_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>



Re: Review Request 63383: Suppress multiline logging from mesos callbacks

2017-10-27 Thread Joshua Cohen


> On Oct. 28, 2017, 12:08 a.m., Jordan Ly wrote:
> > src/main/java/org/apache/aurora/scheduler/mesos/MesosCallbackHandler.java
> > Lines 303-305 (patched)
> > 
> >
> > Does Mesos send any other important information on new lines? I am 
> > mostly worried about losing potentially valuable debugging info.
> 
> Bill Farner wrote:
> Not to my knowledge; historically the message (multi-line or not) has 
> been of little value even for debugging.

Is the info that we're truncating here also available in the Mesos logs 
somewhere?


- Joshua


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63383/#review189497
---


On Oct. 27, 2017, 11:46 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63383/
> ---
> 
> (Updated Oct. 27, 2017, 11:46 p.m.)
> 
> 
> Review request for Aurora and Jordan Ly.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Log messages like this:
> ```console
> W1026 02:55:19.450 [Thread-4822, 
> MesosCallbackHandler$MesosCallbackHandlerImpl] Lost executor value: 
> "thermos-foo-prod-bar-2-40501be8-11fd-417a-8ad3-326c014e8466"
>  on slave value: "efe603c2-8b52-4cc1-b9f4-43a4eb06d29b-S1521"
>  with status 9
> ```
> become:
> ```console
> W1026 02:55:19.450 [Thread-4822, 
> MesosCallbackHandler$MesosCallbackHandlerImpl] Lost executor: 
> "thermos-foo-prod-bar-2-40501be8-11fd-417a-8ad3-326c014e8466" on slave: 
> "efe603c2-8b52-4cc1-b9f4-43a4eb06d29b-S1521" with status 9
> ```
> 
> And messages like this:
> ```console
> I1026 00:00:23.271 [Thread-1152062, 
> MesosCallbackHandler$MesosCallbackHandlerImpl] Received status update for 
> task foo-prod-bar-25-6ac5d5e9-2563-4cd0-a4a8-eb19a990bad4 in state 
> TASK_FAILED from SOURCE_AGENT with REASON_CONTAINER_LIMITATION_MEMORY: Memory 
> limit exceeded: Requested: 14464MB Maximum Used: 14464MB
> 
> MEMORY STATISTICS:
> cache 0
> rss 15166603264
> rss_huge 0
> mapped_file 0
> writeback 0
> pgpgin 6430645
> pgpgout 2727861
> pgfault 6441533
> pgmajfault 4
> inactive_anon 0
> active_anon 15166365696
> inactive_file 0
> active_file 0
> unevictable 0
> hierarchical_memory_limit 15166603264
> total_cache 0
> total_rss 15166603264
> total_rss_huge 0
> total_mapped_file 0
> total_writeback 0
> total_pgpgin 6430645
> total_pgpgout 2727861
> total_pgfault 6441533
> total_pgmajfault 4
> total_inactive_anon 0
> total_active_anon 15166365696
> total_inactive_file 0
> total_active_file 0
> total_unevictable 0
> 
> 
> ```
> 
> become:
> ```console
> I1027 18:48:40.793 [Thread-754, 
> MesosCallbackHandler$MesosCallbackHandlerImpl] Received status update for 
> task foo-prod-bar-11-c39221c6-fd87-40cf-a5a8-555f7bea33f8 in state 
> TASK_FAILED from SOURCE_AGENT with REASON_CONTAINER_LIMITATION_MEMORY: Memory 
> limit exceeded: Requested: 14464MB Maximum Used: 14464MB (truncated)
> ```
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosCallbackHandler.java 
> e93c4fa15a7cc1b025dcb0f29319bc774c62e2c9 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosCallbackHandlerTest.java 
> 51d0371007564bc0af32e256dbae8b57113536c2 
> 
> 
> Diff: https://reviews.apache.org/r/63383/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Review Request 63383: Suppress multiline logging from mesos callbacks

2017-10-27 Thread Bill Farner

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63383/
---

Review request for Aurora and Jordan Ly.


Repository: aurora


Description
---

Log messages like this:
```console
W1026 02:55:19.450 [Thread-4822, MesosCallbackHandler$MesosCallbackHandlerImpl] 
Lost executor value: 
"thermos-tss-production-prod-canary-2-40501be8-11fd-417a-8ad3-326c014e8466"
 on slave value: "efe603c2-8b52-4cc1-b9f4-43a4eb06d29b-S1521"
 with status 9
```
become:
```console
W1026 02:55:19.450 [Thread-4822, MesosCallbackHandler$MesosCallbackHandlerImpl] 
Lost executor: "thermos-foo-prod-bar-2-40501be8-11fd-417a-8ad3-326c014e8466" on 
slave: "efe603c2-8b52-4cc1-b9f4-43a4eb06d29b-S1521" with status 9
```

And messages like this:
```console
I1027 18:48:40.793 [Thread-754, MesosCallbackHandler$MesosCallbackHandlerImpl] 
Received status update for task 
chubbysnipe-staging-aggregator-11-c39221c6-fd87-40cf-a5a8-555f7bea33f8 in state 
TA

MEMORY STATISTICS:
cache 94208
rss 15166509056
rss_huge 0
mapped_file 32768
writeback 0
pgpgin 6195133
pgpgout 2492349
pgfault 6255844
pgmajfault 5
inactive_anon 0
active_anon 15166472192
inactive_file 8192
active_file 8192
unevictable 0
hierarchical_memory_limit 15166603264
total_cache 94208
total_rss 15166509056
total_rss_huge 0
total_mapped_file 32768
total_writeback 0
total_pgpgin 6195133
total_pgpgout 2492349
total_pgfault 6255844
total_pgmajfault 5
total_inactive_anon 0
total_active_anon 15166472192
total_inactive_file 45056
total_active_file 12288
total_unevictable 0
```

become:
```console
I1027 18:48:40.793 [Thread-754, MesosCallbackHandler$MesosCallbackHandlerImpl] 
Received status update for task 
foo-prod-bar-11-c39221c6-fd87-40cf-a5a8-555f7bea33f8 in state TASK_FAILED from 
SOURCE_AGENT with REASON_CONTAINER_LIMITATION_MEMORY: Memory limit exceeded: 
Requested: 14464MB Maximum Used: 14464MB (truncated)
```


Diffs
-

  src/main/java/org/apache/aurora/scheduler/mesos/MesosCallbackHandler.java 
e93c4fa15a7cc1b025dcb0f29319bc774c62e2c9 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosCallbackHandlerTest.java 
51d0371007564bc0af32e256dbae8b57113536c2 


Diff: https://reviews.apache.org/r/63383/diff/1/


Testing
---


Thanks,

Bill Farner



Re: Review Request 63383: Suppress multiline logging from mesos callbacks

2017-10-27 Thread Aurora ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63383/#review189498
---


Ship it!




Master (f6c40a2) 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 Oct. 27, 2017, 11:46 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63383/
> ---
> 
> (Updated Oct. 27, 2017, 11:46 p.m.)
> 
> 
> Review request for Aurora and Jordan Ly.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Log messages like this:
> ```console
> W1026 02:55:19.450 [Thread-4822, 
> MesosCallbackHandler$MesosCallbackHandlerImpl] Lost executor value: 
> "thermos-foo-prod-bar-2-40501be8-11fd-417a-8ad3-326c014e8466"
>  on slave value: "efe603c2-8b52-4cc1-b9f4-43a4eb06d29b-S1521"
>  with status 9
> ```
> become:
> ```console
> W1026 02:55:19.450 [Thread-4822, 
> MesosCallbackHandler$MesosCallbackHandlerImpl] Lost executor: 
> "thermos-foo-prod-bar-2-40501be8-11fd-417a-8ad3-326c014e8466" on slave: 
> "efe603c2-8b52-4cc1-b9f4-43a4eb06d29b-S1521" with status 9
> ```
> 
> And messages like this:
> ```console
> I1026 00:00:23.271 [Thread-1152062, 
> MesosCallbackHandler$MesosCallbackHandlerImpl] Received status update for 
> task foo-prod-bar-25-6ac5d5e9-2563-4cd0-a4a8-eb19a990bad4 in state 
> TASK_FAILED from SOURCE_AGENT with REASON_CONTAINER_LIMITATION_MEMORY: Memory 
> limit exceeded: Requested: 14464MB Maximum Used: 14464MB
> 
> MEMORY STATISTICS:
> cache 0
> rss 15166603264
> rss_huge 0
> mapped_file 0
> writeback 0
> pgpgin 6430645
> pgpgout 2727861
> pgfault 6441533
> pgmajfault 4
> inactive_anon 0
> active_anon 15166365696
> inactive_file 0
> active_file 0
> unevictable 0
> hierarchical_memory_limit 15166603264
> total_cache 0
> total_rss 15166603264
> total_rss_huge 0
> total_mapped_file 0
> total_writeback 0
> total_pgpgin 6430645
> total_pgpgout 2727861
> total_pgfault 6441533
> total_pgmajfault 4
> total_inactive_anon 0
> total_active_anon 15166365696
> total_inactive_file 0
> total_active_file 0
> total_unevictable 0
> 
> 
> ```
> 
> become:
> ```console
> I1027 18:48:40.793 [Thread-754, 
> MesosCallbackHandler$MesosCallbackHandlerImpl] Received status update for 
> task foo-prod-bar-11-c39221c6-fd87-40cf-a5a8-555f7bea33f8 in state 
> TASK_FAILED from SOURCE_AGENT with REASON_CONTAINER_LIMITATION_MEMORY: Memory 
> limit exceeded: Requested: 14464MB Maximum Used: 14464MB (truncated)
> ```
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosCallbackHandler.java 
> e93c4fa15a7cc1b025dcb0f29319bc774c62e2c9 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosCallbackHandlerTest.java 
> 51d0371007564bc0af32e256dbae8b57113536c2 
> 
> 
> Diff: https://reviews.apache.org/r/63383/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 63383: Suppress multiline logging from mesos callbacks

2017-10-27 Thread Jordan Ly

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63383/#review189497
---


Fix it, then Ship it!




One small question, but overall LGTM.


src/main/java/org/apache/aurora/scheduler/mesos/MesosCallbackHandler.java
Lines 303-305 (patched)


Does Mesos send any other important information on new lines? I am mostly 
worried about losing potentially valuable debugging info.


- Jordan Ly


On Oct. 27, 2017, 11:46 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63383/
> ---
> 
> (Updated Oct. 27, 2017, 11:46 p.m.)
> 
> 
> Review request for Aurora and Jordan Ly.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Log messages like this:
> ```console
> W1026 02:55:19.450 [Thread-4822, 
> MesosCallbackHandler$MesosCallbackHandlerImpl] Lost executor value: 
> "thermos-foo-prod-bar-2-40501be8-11fd-417a-8ad3-326c014e8466"
>  on slave value: "efe603c2-8b52-4cc1-b9f4-43a4eb06d29b-S1521"
>  with status 9
> ```
> become:
> ```console
> W1026 02:55:19.450 [Thread-4822, 
> MesosCallbackHandler$MesosCallbackHandlerImpl] Lost executor: 
> "thermos-foo-prod-bar-2-40501be8-11fd-417a-8ad3-326c014e8466" on slave: 
> "efe603c2-8b52-4cc1-b9f4-43a4eb06d29b-S1521" with status 9
> ```
> 
> And messages like this:
> ```console
> I1026 00:00:23.271 [Thread-1152062, 
> MesosCallbackHandler$MesosCallbackHandlerImpl] Received status update for 
> task foo-prod-bar-25-6ac5d5e9-2563-4cd0-a4a8-eb19a990bad4 in state 
> TASK_FAILED from SOURCE_AGENT with REASON_CONTAINER_LIMITATION_MEMORY: Memory 
> limit exceeded: Requested: 14464MB Maximum Used: 14464MB
> 
> MEMORY STATISTICS:
> cache 0
> rss 15166603264
> rss_huge 0
> mapped_file 0
> writeback 0
> pgpgin 6430645
> pgpgout 2727861
> pgfault 6441533
> pgmajfault 4
> inactive_anon 0
> active_anon 15166365696
> inactive_file 0
> active_file 0
> unevictable 0
> hierarchical_memory_limit 15166603264
> total_cache 0
> total_rss 15166603264
> total_rss_huge 0
> total_mapped_file 0
> total_writeback 0
> total_pgpgin 6430645
> total_pgpgout 2727861
> total_pgfault 6441533
> total_pgmajfault 4
> total_inactive_anon 0
> total_active_anon 15166365696
> total_inactive_file 0
> total_active_file 0
> total_unevictable 0
> 
> 
> ```
> 
> become:
> ```console
> I1027 18:48:40.793 [Thread-754, 
> MesosCallbackHandler$MesosCallbackHandlerImpl] Received status update for 
> task foo-prod-bar-11-c39221c6-fd87-40cf-a5a8-555f7bea33f8 in state 
> TASK_FAILED from SOURCE_AGENT with REASON_CONTAINER_LIMITATION_MEMORY: Memory 
> limit exceeded: Requested: 14464MB Maximum Used: 14464MB (truncated)
> ```
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosCallbackHandler.java 
> e93c4fa15a7cc1b025dcb0f29319bc774c62e2c9 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosCallbackHandlerTest.java 
> 51d0371007564bc0af32e256dbae8b57113536c2 
> 
> 
> Diff: https://reviews.apache.org/r/63383/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 63383: Suppress multiline logging from mesos callbacks

2017-10-27 Thread Bill Farner


> On Oct. 27, 2017, 5:08 p.m., Jordan Ly wrote:
> > src/main/java/org/apache/aurora/scheduler/mesos/MesosCallbackHandler.java
> > Lines 303-305 (patched)
> > 
> >
> > Does Mesos send any other important information on new lines? I am 
> > mostly worried about losing potentially valuable debugging info.

Not to my knowledge; historically the message (multi-line or not) has been of 
little value even for debugging.


- Bill


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63383/#review189497
---


On Oct. 27, 2017, 4:46 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63383/
> ---
> 
> (Updated Oct. 27, 2017, 4:46 p.m.)
> 
> 
> Review request for Aurora and Jordan Ly.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Log messages like this:
> ```console
> W1026 02:55:19.450 [Thread-4822, 
> MesosCallbackHandler$MesosCallbackHandlerImpl] Lost executor value: 
> "thermos-foo-prod-bar-2-40501be8-11fd-417a-8ad3-326c014e8466"
>  on slave value: "efe603c2-8b52-4cc1-b9f4-43a4eb06d29b-S1521"
>  with status 9
> ```
> become:
> ```console
> W1026 02:55:19.450 [Thread-4822, 
> MesosCallbackHandler$MesosCallbackHandlerImpl] Lost executor: 
> "thermos-foo-prod-bar-2-40501be8-11fd-417a-8ad3-326c014e8466" on slave: 
> "efe603c2-8b52-4cc1-b9f4-43a4eb06d29b-S1521" with status 9
> ```
> 
> And messages like this:
> ```console
> I1026 00:00:23.271 [Thread-1152062, 
> MesosCallbackHandler$MesosCallbackHandlerImpl] Received status update for 
> task foo-prod-bar-25-6ac5d5e9-2563-4cd0-a4a8-eb19a990bad4 in state 
> TASK_FAILED from SOURCE_AGENT with REASON_CONTAINER_LIMITATION_MEMORY: Memory 
> limit exceeded: Requested: 14464MB Maximum Used: 14464MB
> 
> MEMORY STATISTICS:
> cache 0
> rss 15166603264
> rss_huge 0
> mapped_file 0
> writeback 0
> pgpgin 6430645
> pgpgout 2727861
> pgfault 6441533
> pgmajfault 4
> inactive_anon 0
> active_anon 15166365696
> inactive_file 0
> active_file 0
> unevictable 0
> hierarchical_memory_limit 15166603264
> total_cache 0
> total_rss 15166603264
> total_rss_huge 0
> total_mapped_file 0
> total_writeback 0
> total_pgpgin 6430645
> total_pgpgout 2727861
> total_pgfault 6441533
> total_pgmajfault 4
> total_inactive_anon 0
> total_active_anon 15166365696
> total_inactive_file 0
> total_active_file 0
> total_unevictable 0
> 
> 
> ```
> 
> become:
> ```console
> I1027 18:48:40.793 [Thread-754, 
> MesosCallbackHandler$MesosCallbackHandlerImpl] Received status update for 
> task foo-prod-bar-11-c39221c6-fd87-40cf-a5a8-555f7bea33f8 in state 
> TASK_FAILED from SOURCE_AGENT with REASON_CONTAINER_LIMITATION_MEMORY: Memory 
> limit exceeded: Requested: 14464MB Maximum Used: 14464MB (truncated)
> ```
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosCallbackHandler.java 
> e93c4fa15a7cc1b025dcb0f29319bc774c62e2c9 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosCallbackHandlerTest.java 
> 51d0371007564bc0af32e256dbae8b57113536c2 
> 
> 
> Diff: https://reviews.apache.org/r/63383/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 63374: Revert to old Job Page tab names and add counts

2017-10-27 Thread Kai Huang

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63374/#review189477
---


Ship it!




Ship It!

- Kai Huang


On Oct. 27, 2017, 7:48 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63374/
> ---
> 
> (Updated Oct. 27, 2017, 7:48 p.m.)
> 
> 
> Review request for Aurora, Kai Huang and Reza Motamedi.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Changing the names of tabs causes unnecessary confusion. Revert to "active 
> tasks/completed tasks" and add the instance count back to both.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/JobHistory.js 
> 74f6fcb1117e80bcd5a033dad06ca00f9bc2ddfc 
>   ui/src/main/js/components/JobStatus.js 
> 84e335c91da7231eda30ae945dd669f80fb1cc38 
> 
> 
> Diff: https://reviews.apache.org/r/63374/diff/1/
> 
> 
> Testing
> ---
> 
> See screenshots.
> 
> 
> File Attachments
> 
> 
> Screen Shot 2017-10-27 at 12.46.52 PM.png
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/27/33061824-a52c-4b54-a14f-27793f5bc9ea__Screen_Shot_2017-10-27_at_12.46.52_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>



Re: Review Request 63316: MesosCallbackHandler uses separate eventbus for registered call

2017-10-27 Thread Jordan Ly

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63316/
---

(Updated Oct. 27, 2017, 9:13 p.m.)


Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner.


Changes
---

Switch to pubsub implementation.


Summary (updated)
-

MesosCallbackHandler uses separate eventbus for registered call


Bugs: AURORA-1953
https://issues.apache.org/jira/browse/AURORA-1953


Repository: aurora


Description (updated)
---

We should have `registered` use its own eventbus so it does not get blocked by 
other calls.

See attached ticket for details.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/SchedulerModule.java 
382181939d292ee00ea2071daec66aa0669609e4 
  src/main/java/org/apache/aurora/scheduler/events/PubsubEventModule.java 
6758f8c3d6fbfb9f3e6f4d05d27a423388d0bdab 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosCallbackHandler.java 
68d19ec53b3bb5ee02934254e8f9c82ae720c6ed 
  src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverModule.java 
b54e1f3d427b8e99fb022536004014c3fee498d0 
  src/test/java/org/apache/aurora/scheduler/events/PubsubEventModuleTest.java 
f656b271c0f3270f162c205923b300d8fe1ec0b7 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosCallbackHandlerTest.java 
4d1a6761b8cd761d96cbfb35b0944eb524bd654f 


Diff: https://reviews.apache.org/r/63316/diff/6/

Changes: https://reviews.apache.org/r/63316/diff/5-6/


Testing
---

`./gradlew test`


Thanks,

Jordan Ly



Re: Review Request 63316: MesosCallbackHandler uses separate eventbus for registered call

2017-10-27 Thread Aurora ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63316/#review189485
---



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

 [182] ./~/bootstrap/dist/css/bootstrap.css 984 bytes {0} [built]
 [183] ./src/main/sass/app.scss 1.2 kB {0} [built]
 [184] ./src/main/resources/source-sans-pro.css 1.05 kB {0} [built]
 [216] ./src/main/js/index.js 3.15 kB {0} [built]
 [253] ./~/react-router-dom/es/Prompt.js 131 bytes {0} [built]
+ 263 hidden modules
:processResources
:classes
:jar
:startScripts
:distTar
:distZip
:assemble
:compileTestJavaNote: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.

:processTestResources
:testClasses
:compileJmhJavaNote: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/jmh/java/org/apache/aurora/benchmark/fakes/FakeSchedulerDriver.java
 uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.

:processJmhResources NO-SOURCE
:jmhClasses
:checkstyleJmh
:checkstyleMain[ant:checkstyle] [ERROR] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/events/PubsubEventModule.java:35:8:
 Unused import - com.google.inject.Key. [UnusedImports]
 FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':checkstyleMain'.
> Checkstyle rule violations were found. See the report at: 
> file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/checkstyle/main.html

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.

* Get more help at https://help.gradle.org

BUILD FAILED in 4m 7s
36 actionable tasks: 30 executed, 6 up-to-date


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

- Aurora ReviewBot


On Oct. 27, 2017, 9:13 p.m., Jordan Ly wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63316/
> ---
> 
> (Updated Oct. 27, 2017, 9:13 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner.
> 
> 
> Bugs: AURORA-1953
> https://issues.apache.org/jira/browse/AURORA-1953
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> We should have `registered` use its own eventbus so it does not get blocked 
> by other calls.
> 
> See attached ticket for details.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/SchedulerModule.java 
> 382181939d292ee00ea2071daec66aa0669609e4 
>   src/main/java/org/apache/aurora/scheduler/events/PubsubEventModule.java 
> 6758f8c3d6fbfb9f3e6f4d05d27a423388d0bdab 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosCallbackHandler.java 
> 68d19ec53b3bb5ee02934254e8f9c82ae720c6ed 
>   src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverModule.java 
> b54e1f3d427b8e99fb022536004014c3fee498d0 
>   src/test/java/org/apache/aurora/scheduler/events/PubsubEventModuleTest.java 
> f656b271c0f3270f162c205923b300d8fe1ec0b7 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosCallbackHandlerTest.java 
> 4d1a6761b8cd761d96cbfb35b0944eb524bd654f 
> 
> 
> Diff: https://reviews.apache.org/r/63316/diff/6/
> 
> 
> Testing
> ---
> 
> `./gradlew test`
> 
> 
> Thanks,
> 
> Jordan Ly
> 
>



Re: Review Request 63316: MesosCallbackHandler uses separate eventbus for registered call

2017-10-27 Thread Aurora ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63316/#review189487
---



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

:processJmhResources NO-SOURCE
:jmhClasses
:checkstyleJmh
:checkstyleMain
:checkstyleTest
:findbugsJmh
:findbugsMain
:findbugsTest
:licenseJmh UP-TO-DATE
:licenseMain UP-TO-DATE
:licenseTest UP-TO-DATE
:license UP-TO-DATE
:pmdJmh
:pmdMain
:pmdTest
:test

org.apache.aurora.scheduler.events.WebhookTest > 
testTaskChangedWithOldStateError FAILED
java.lang.AssertionError at WebhookTest.java:203
I1027 21:38:06.035 [ShutdownHook, SchedulerMain] Stopping scheduler services. 

1178 tests completed, 1 failed, 2 skipped
:test FAILED
:jacocoTestReport
Coverage report generated: 
file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/jacoco/test/html/index.html
:jacocoTestCoverageVerification

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':test'.
> There were failing tests. See the report at: 
> file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/tests/test/index.html

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.

* Get more help at https://help.gradle.org

BUILD FAILED in 6m 52s
49 actionable tasks: 40 executed, 9 up-to-date


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

- Aurora ReviewBot


On Oct. 27, 2017, 9:27 p.m., Jordan Ly wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63316/
> ---
> 
> (Updated Oct. 27, 2017, 9:27 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner.
> 
> 
> Bugs: AURORA-1953
> https://issues.apache.org/jira/browse/AURORA-1953
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> We should have `registered` use its own eventbus so it does not get blocked 
> by other calls.
> 
> See attached ticket for details.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/SchedulerModule.java 
> 382181939d292ee00ea2071daec66aa0669609e4 
>   src/main/java/org/apache/aurora/scheduler/events/PubsubEventModule.java 
> 6758f8c3d6fbfb9f3e6f4d05d27a423388d0bdab 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosCallbackHandler.java 
> 68d19ec53b3bb5ee02934254e8f9c82ae720c6ed 
>   src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverModule.java 
> b54e1f3d427b8e99fb022536004014c3fee498d0 
>   src/test/java/org/apache/aurora/scheduler/events/PubsubEventModuleTest.java 
> f656b271c0f3270f162c205923b300d8fe1ec0b7 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosCallbackHandlerTest.java 
> 4d1a6761b8cd761d96cbfb35b0944eb524bd654f 
> 
> 
> Diff: https://reviews.apache.org/r/63316/diff/7/
> 
> 
> Testing
> ---
> 
> `./gradlew test`
> 
> 
> Thanks,
> 
> Jordan Ly
> 
>



Re: Review Request 63316: MesosCallbackHandler uses separate eventbus for registered call

2017-10-27 Thread Jordan Ly

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63316/
---

(Updated Oct. 27, 2017, 9:27 p.m.)


Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner.


Changes
---

Remove unused import


Bugs: AURORA-1953
https://issues.apache.org/jira/browse/AURORA-1953


Repository: aurora


Description
---

We should have `registered` use its own eventbus so it does not get blocked by 
other calls.

See attached ticket for details.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/SchedulerModule.java 
382181939d292ee00ea2071daec66aa0669609e4 
  src/main/java/org/apache/aurora/scheduler/events/PubsubEventModule.java 
6758f8c3d6fbfb9f3e6f4d05d27a423388d0bdab 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosCallbackHandler.java 
68d19ec53b3bb5ee02934254e8f9c82ae720c6ed 
  src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverModule.java 
b54e1f3d427b8e99fb022536004014c3fee498d0 
  src/test/java/org/apache/aurora/scheduler/events/PubsubEventModuleTest.java 
f656b271c0f3270f162c205923b300d8fe1ec0b7 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosCallbackHandlerTest.java 
4d1a6761b8cd761d96cbfb35b0944eb524bd654f 


Diff: https://reviews.apache.org/r/63316/diff/7/

Changes: https://reviews.apache.org/r/63316/diff/6-7/


Testing
---

`./gradlew test`


Thanks,

Jordan Ly



Re: Review Request 63316: MesosCallbackHandler uses separate eventbus for registered call

2017-10-27 Thread Bill Farner

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63316/#review189482
---


Ship it!




Ship It!

- Bill Farner


On Oct. 27, 2017, 2:13 p.m., Jordan Ly wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63316/
> ---
> 
> (Updated Oct. 27, 2017, 2:13 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner.
> 
> 
> Bugs: AURORA-1953
> https://issues.apache.org/jira/browse/AURORA-1953
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> We should have `registered` use its own eventbus so it does not get blocked 
> by other calls.
> 
> See attached ticket for details.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/SchedulerModule.java 
> 382181939d292ee00ea2071daec66aa0669609e4 
>   src/main/java/org/apache/aurora/scheduler/events/PubsubEventModule.java 
> 6758f8c3d6fbfb9f3e6f4d05d27a423388d0bdab 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosCallbackHandler.java 
> 68d19ec53b3bb5ee02934254e8f9c82ae720c6ed 
>   src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverModule.java 
> b54e1f3d427b8e99fb022536004014c3fee498d0 
>   src/test/java/org/apache/aurora/scheduler/events/PubsubEventModuleTest.java 
> f656b271c0f3270f162c205923b300d8fe1ec0b7 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosCallbackHandlerTest.java 
> 4d1a6761b8cd761d96cbfb35b0944eb524bd654f 
> 
> 
> Diff: https://reviews.apache.org/r/63316/diff/6/
> 
> 
> Testing
> ---
> 
> `./gradlew test`
> 
> 
> Thanks,
> 
> Jordan Ly
> 
>



Re: Review Request 63199: Refactor staticallyBannedOffers into a LRU cache

2017-10-27 Thread Bill Farner

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63199/#review189488
---




src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java
Lines 321 (patched)


`expireAfterWrite` doesn't result in LRU.  I think you mean 
`expireAfterAccess`.



src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java
Lines 42-43 (patched)


How about a `CacheBuilderSpec` to bundle these?



src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java
Lines 46-50 (patched)


We've developed a 
[stutter](https://michaelwhatcott.com/go-code-that-stutters/) over time in this 
class.  Can you do some cleanup and s/offer// on parameters, fields, and 
methods?



src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java
Lines 104 (patched)


`0` is a valid size for guava's `CacheBuilder#maximumSize`:
> When size is zero, elements will be evicted immediately after being 
loaded into the cache. This can be useful in testing, or to disable caching 
temporarily without a code change.

Let's allow it, and introduce a `NotNegativeNumber` counterpart to 
`PositiveNumber`.



src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java
Lines 109 (patched)


s/Long/long/



src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java
Line 178 (original), 188-189 (patched)


Is this needed only because of the default `offerStaticBanCacheMaxSize = 
Long.MAX_VALUE`?  The double eviction strategy intuitively seems unnecessary.  
I suggest a finite default for `offerStaticBanCacheMaxSize` (say, 1k) and no 
time expiry.



src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java
Lines 247-249 (original), 253-255 (patched)


IMHO this test case is no longer interesting now that we have to punch 
through with `cleanupStaticBans()`.  The test now reads as "test that clearing 
a cache clears the cache".  I suggest removing the test case.


- Bill Farner


On Oct. 20, 2017, 10:53 p.m., Jordan Ly wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63199/
> ---
> 
> (Updated Oct. 20, 2017, 10:53 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, 
> Stephan Erb, and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Using the new `hold_offers_forever` option, it is possible for the 
> `staticallyBannedOffers` to grow very large in size as we never release 
> offers.
> 
> As an alternative to https://reviews.apache.org/r/63121/, I propose changing 
> `staticallyBannedOffers` into a LRU cache which expires entries after 
> `min_offer_hold_time` + `offer_hold_jitter_window` (referred to as 
> `maxOfferHoldTime`), while also taking an option for a maximum size for the 
> cache. I believe that this approach has a couple of benefits:
> 
> 1. The current behavior of `staticallyBannedOffers` is (kinda) preserved. 
> Entries will no longer be removed when the offer is used, but they will be 
> removed within `maxOfferHoldTime`. This means cluster operators will not have 
> to think about the new `offer_static_ban_cache_max_size` if they aren't 
> affected by the memory leak now.
> 2. Cluster operators that use Aurora as a single framework and hold offers 
> indefinitely can cap the size of the cache to avoid the memory leak.
> 3. Using an LRU cache greatly benefits quickly recurring crons and job 
> updates.
> 
> 
> Diffs
> -
> 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 5a9099bf9dd292249d72bc3a7604fbb3394f30ea 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 
> 7011a4cc9eea827cdd54698aaed1a653774bce7f 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 
> e060f2073dce4d2486d1ee2bfd873fe75167c6d0 
>   src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java 
> e6b2c55e4f33f9a603157236766425edcaff10e7 
>   src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java 
> 5b502442163581daa4d7954b09c00bdc3680a726 
>   src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 
> 6c8434e9cfe46ef63ff10c6f059ecb99981f29a2 
> 
> 
> Diff: https://reviews.apache.org/r/63199/diff/4/
> 
> 
> Testing
> ---
> 
> Unit tests pass.
> Deployed on a scale test cluster and saw that a) `staticallyBannedOffers` 
> memory leak fixed with 

Re: Review Request 63375: Add resource units to config summary

2017-10-27 Thread Reza Motamedi

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63375/#review189483
---


Ship it!




Ship It!

- Reza Motamedi


On Oct. 27, 2017, 7:58 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63375/
> ---
> 
> (Updated Oct. 27, 2017, 7:58 p.m.)
> 
> 
> Review request for Aurora, Kai Huang and Reza Motamedi.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add resource units to config summary
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/RoleQuota.js 
> bb1b6e7e5dbedf875f27caf9bc9ca28319b82c5b 
>   ui/src/main/js/components/TaskConfigSummary.js 
> b1cf5a956e0230932944f76ad8d25b356b9d8f23 
>   ui/src/main/js/utils/Quota.js PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63375/diff/1/
> 
> 
> Testing
> ---
> 
> See screenshot.
> 
> 
> File Attachments
> 
> 
> Screen Shot 2017-10-27 at 12.55.52 PM.png
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/27/be64586c-3ab8-4e17-bec2-b166719da2e7__Screen_Shot_2017-10-27_at_12.55.52_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>