Re: Review Request 52074: switching from launchTask to acceptOffers

2016-09-19 Thread Aurora ReviewBot 

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



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

:commons:processResources
:commons:classes
:commons:jar
:compileJava/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java:74:
 Note: Wrote forwarder 
org.apache.aurora.scheduler.storage.log.WriteAheadStorageForwarder
@Forward({
^
Note: Writing 
file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/org/apache/aurora/common/args/apt/cmdline.arg.info.txt.2
Note: Writing 
file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/META-INF/compiler/resource-mappings/org.apache.aurora.common.args.apt.CmdLineProcessor

:generateBuildProperties
:processResources
:classes
:jar
:startScripts
:distTar
:distZip
:assemble
:compileJmhJava/home/jenkins/jenkins-slave/workspace/AuroraBot/src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java:23:
 error: FakeDriver is not abstract and does not override abstract method 
acceptOffers(OfferID,Collection,Filters) in Driver
public class FakeDriver extends AbstractIdleService implements Driver {
   ^
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java:29:
 error: method does not override or implement a method from a supertype
  @Override
  ^
Note: 
/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.
2 errors
 FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':compileJmhJava'.
> Compilation failed; see the compiler error output for details.

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

BUILD FAILED

Total time: 2 mins 2.425 secs


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

- Aurora ReviewBot


On Sept. 20, 2016, 2:12 a.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52074/
> ---
> 
> (Updated Sept. 20, 2016, 2:12 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko, Mehrdad Nurolahzade, and Zameer 
> Manji.
> 
> 
> Bugs: AURORA-1776
> https://issues.apache.org/jira/browse/AURORA-1776
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Dynamic reservation requires us to use acceptOffers so this is in preparation 
> of that work. acceptOffers is the call to dynamically reserve resources.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/mesos/Driver.java 
> 14481468926b82a9bad3ee78c85f176e8ba893e2 
>   src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java 
> 41b9aab55aa439b13eba3545703ec97b44690444 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 
> 925c025d21bf1d44e0c1d319f6653ecfa8899481 
>   src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 
> a739bce1226d9435fa7d0b18e411064a4e78e49e 
> 
> Diff: https://reviews.apache.org/r/52074/diff/
> 
> 
> Testing
> ---
> 
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>

Review Request 52074: switching from launchTask to acceptOffers

2016-09-19 Thread Dmitriy Shirchenko 

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

Review request for Aurora, Maxim Khutornenko, Mehrdad Nurolahzade, and Zameer 
Manji.


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


Repository: aurora


Description
---

Dynamic reservation requires us to use acceptOffers so this is in preparation 
of that work. acceptOffers is the call to dynamically reserve resources.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/mesos/Driver.java 
14481468926b82a9bad3ee78c85f176e8ba893e2 
  src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java 
41b9aab55aa439b13eba3545703ec97b44690444 
  src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 
925c025d21bf1d44e0c1d319f6653ecfa8899481 
  src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 
a739bce1226d9435fa7d0b18e411064a4e78e49e 

Diff: https://reviews.apache.org/r/52074/diff/


Testing
---

src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh


Thanks,

Dmitriy Shirchenko

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

2016-09-19 Thread Aurora ReviewBot 

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


Ship it!




Master (4c4040f) 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 Sept. 20, 2016, 12:25 a.m., Kai Huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51876/
> ---
> 
> (Updated Sept. 20, 2016, 12:25 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.
> 
> 
> Bugs: AURORA-1225
> https://issues.apache.org/jira/browse/AURORA-1225
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Modify executor state transition logic to rely on health checks (if enabled).
> 
> [Summary]
> Executor needs to start executing user content in STARTING and transition to 
> RUNNING when a successful required number of health checks is reached.
> 
> This review contains a series of executor changes that implement the health 
> check driven updates. It gives more context of the design of this feature.
> 
> [Background]
> Please see this epic: https://issues.apache.org/jira/browse/AURORA-1225
> and the design doc: 
> https://docs.google.com/document/d/1ZdgW8S4xMhvKW7iQUX99xZm10NXSxEWR0a-21FP5d94/edit#
>  for more details and background.
> 
> [Description]
> If health check is enabled on vCurrent executor, the health checker will send 
> a "TASK_RUNNING" message when a successful required number of health checks 
> is reached within the initial_interval_secs. On the other hand, a 
> "TASK_FAILED" message was sent if the health checker fails to reach the 
> required number of health checks within that period, or a maximum number of 
> failed health check limit is reached after the initital_interval_secs.
> 
> If health check is disabled on the vCurrent executor, it will sends 
> "TASK_RUNNING" message to scheduler after the thermos runner was started. In 
> this scenario, the behavior of vCurrent executor will be the same as the 
> vPrev executor.
> 
> [Change List]
> The current change set includes:
> 1. Removed the status memoization in ChainedStatusChecker.
> 2. Modified the StatusManager to be edge triggered.
> 3. Changed the Aurora Executor callback function.
> 4. Modified the Health Checker and redefined the meaning 
> initial_interval_secs.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> c5765b70501c101f0535b4eed94e9948c36808f9 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> ce5ef680f01831cd89fced8969ae3246c7f60cfd 
>   src/main/python/apache/aurora/executor/common/health_checker.py 
> 5fc845eceac6f0c048d7489fdc4c672b0c609ea0 
>   src/main/python/apache/aurora/executor/common/status_checker.py 
> 795dae2d6b661fc528d952c2315196d94127961f 
>   src/main/python/apache/aurora/executor/common/task_info.py 
> 4ef49e30eeb942886d02c1fb448055264f9aa874 
>   src/main/python/apache/aurora/executor/status_manager.py 
> 228a99a05f339e21cd7e769a42b9b2276e7bc3fc 
>   src/test/python/apache/aurora/executor/common/test_health_checker.py 
> bb6ea69dd94298c5b8cf4d5f06d06eea7790d66e 
>   src/test/python/apache/aurora/executor/common/test_status_checker.py 
> 5be1981c8c8e88258456adb21aa3ca7c0aa472a7 
>   src/test/python/apache/aurora/executor/common/test_task_info.py 
> 0d9cc5cb340d697a887d8e001f23c948f4fa70c7 
>   src/test/python/apache/aurora/executor/test_status_manager.py 
> ce4679ba1aa7b42cf0115c943d84663030182d23 
>   src/test/python/apache/aurora/executor/test_thermos_executor.py 
> 0bfe9e931f873c9f804f2ba4012e050e1f9fd24e 
> 
> Diff: https://reviews.apache.org/r/51876/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> 
> ./pants test.pytest src/test/python/apache/aurora/executor::
> 
> 
> Thanks,
> 
> Kai Huang
> 
>

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

2016-09-19 Thread Kai Huang 

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




src/main/python/apache/aurora/executor/common/status_checker.py (line 104)


Use TaskState.Value('TASK_RUNNING') here instead of mesos_pb2.TASK_RUNNING, 
because this file also used TaskState in multiple places.


- Kai Huang


On Sept. 20, 2016, 12:25 a.m., Kai Huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51876/
> ---
> 
> (Updated Sept. 20, 2016, 12:25 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.
> 
> 
> Bugs: AURORA-1225
> https://issues.apache.org/jira/browse/AURORA-1225
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Modify executor state transition logic to rely on health checks (if enabled).
> 
> [Summary]
> Executor needs to start executing user content in STARTING and transition to 
> RUNNING when a successful required number of health checks is reached.
> 
> This review contains a series of executor changes that implement the health 
> check driven updates. It gives more context of the design of this feature.
> 
> [Background]
> Please see this epic: https://issues.apache.org/jira/browse/AURORA-1225
> and the design doc: 
> https://docs.google.com/document/d/1ZdgW8S4xMhvKW7iQUX99xZm10NXSxEWR0a-21FP5d94/edit#
>  for more details and background.
> 
> [Description]
> If health check is enabled on vCurrent executor, the health checker will send 
> a "TASK_RUNNING" message when a successful required number of health checks 
> is reached within the initial_interval_secs. On the other hand, a 
> "TASK_FAILED" message was sent if the health checker fails to reach the 
> required number of health checks within that period, or a maximum number of 
> failed health check limit is reached after the initital_interval_secs.
> 
> If health check is disabled on the vCurrent executor, it will sends 
> "TASK_RUNNING" message to scheduler after the thermos runner was started. In 
> this scenario, the behavior of vCurrent executor will be the same as the 
> vPrev executor.
> 
> [Change List]
> The current change set includes:
> 1. Removed the status memoization in ChainedStatusChecker.
> 2. Modified the StatusManager to be edge triggered.
> 3. Changed the Aurora Executor callback function.
> 4. Modified the Health Checker and redefined the meaning 
> initial_interval_secs.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> c5765b70501c101f0535b4eed94e9948c36808f9 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> ce5ef680f01831cd89fced8969ae3246c7f60cfd 
>   src/main/python/apache/aurora/executor/common/health_checker.py 
> 5fc845eceac6f0c048d7489fdc4c672b0c609ea0 
>   src/main/python/apache/aurora/executor/common/status_checker.py 
> 795dae2d6b661fc528d952c2315196d94127961f 
>   src/main/python/apache/aurora/executor/common/task_info.py 
> 4ef49e30eeb942886d02c1fb448055264f9aa874 
>   src/main/python/apache/aurora/executor/status_manager.py 
> 228a99a05f339e21cd7e769a42b9b2276e7bc3fc 
>   src/test/python/apache/aurora/executor/common/test_health_checker.py 
> bb6ea69dd94298c5b8cf4d5f06d06eea7790d66e 
>   src/test/python/apache/aurora/executor/common/test_status_checker.py 
> 5be1981c8c8e88258456adb21aa3ca7c0aa472a7 
>   src/test/python/apache/aurora/executor/common/test_task_info.py 
> 0d9cc5cb340d697a887d8e001f23c948f4fa70c7 
>   src/test/python/apache/aurora/executor/test_status_manager.py 
> ce4679ba1aa7b42cf0115c943d84663030182d23 
>   src/test/python/apache/aurora/executor/test_thermos_executor.py 
> 0bfe9e931f873c9f804f2ba4012e050e1f9fd24e 
> 
> Diff: https://reviews.apache.org/r/51876/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> 
> ./pants test.pytest src/test/python/apache/aurora/executor::
> 
> 
> Thanks,
> 
> Kai Huang
> 
>

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

2016-09-19 Thread Kai Huang 

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

(Updated Sept. 20, 2016, 12:25 a.m.)


Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.


Changes
---

Fixed one flaky test for Thermos Executor. Added unit tests for the modified 
components in the notification chain.


Summary (updated)
-

Modify executor state transition logic to rely on health checks (if enabled)


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


Repository: aurora


Description (updated)
---

Modify executor state transition logic to rely on health checks (if enabled).

[Summary]
Executor needs to start executing user content in STARTING and transition to 
RUNNING when a successful required number of health checks is reached.

This review contains a series of executor changes that implement the health 
check driven updates. It gives more context of the design of this feature.

[Background]
Please see this epic: https://issues.apache.org/jira/browse/AURORA-1225
and the design doc: 
https://docs.google.com/document/d/1ZdgW8S4xMhvKW7iQUX99xZm10NXSxEWR0a-21FP5d94/edit#
 for more details and background.

[Description]
If health check is enabled on vCurrent executor, the health checker will send a 
"TASK_RUNNING" message when a successful required number of health checks is 
reached within the initial_interval_secs. On the other hand, a "TASK_FAILED" 
message was sent if the health checker fails to reach the required number of 
health checks within that period, or a maximum number of failed health check 
limit is reached after the initital_interval_secs.

If health check is disabled on the vCurrent executor, it will sends 
"TASK_RUNNING" message to scheduler after the thermos runner was started. In 
this scenario, the behavior of vCurrent executor will be the same as the vPrev 
executor.

[Change List]
The current change set includes:
1. Removed the status memoization in ChainedStatusChecker.
2. Modified the StatusManager to be edge triggered.
3. Changed the Aurora Executor callback function.
4. Modified the Health Checker and redefined the meaning initial_interval_secs.


Diffs (updated)
-

  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
c5765b70501c101f0535b4eed94e9948c36808f9 
  src/main/python/apache/aurora/executor/aurora_executor.py 
ce5ef680f01831cd89fced8969ae3246c7f60cfd 
  src/main/python/apache/aurora/executor/common/health_checker.py 
5fc845eceac6f0c048d7489fdc4c672b0c609ea0 
  src/main/python/apache/aurora/executor/common/status_checker.py 
795dae2d6b661fc528d952c2315196d94127961f 
  src/main/python/apache/aurora/executor/common/task_info.py 
4ef49e30eeb942886d02c1fb448055264f9aa874 
  src/main/python/apache/aurora/executor/status_manager.py 
228a99a05f339e21cd7e769a42b9b2276e7bc3fc 
  src/test/python/apache/aurora/executor/common/test_health_checker.py 
bb6ea69dd94298c5b8cf4d5f06d06eea7790d66e 
  src/test/python/apache/aurora/executor/common/test_status_checker.py 
5be1981c8c8e88258456adb21aa3ca7c0aa472a7 
  src/test/python/apache/aurora/executor/common/test_task_info.py 
0d9cc5cb340d697a887d8e001f23c948f4fa70c7 
  src/test/python/apache/aurora/executor/test_status_manager.py 
ce4679ba1aa7b42cf0115c943d84663030182d23 
  src/test/python/apache/aurora/executor/test_thermos_executor.py 
0bfe9e931f873c9f804f2ba4012e050e1f9fd24e 

Diff: https://reviews.apache.org/r/51876/diff/


Testing
---

./build-support/jenkins/build.sh

./pants test.pytest src/test/python/apache/aurora/executor::


Thanks,

Kai Huang

Re: Review Request 51980: Refactor of Webhook and no longer posting entire task state via webhook on scheduler restart

2016-09-19 Thread Aurora ReviewBot 

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


Ship it!




Master (4c4040f) 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 Sept. 19, 2016, 6:28 p.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51980/
> ---
> 
> (Updated Sept. 19, 2016, 6:28 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1772
> https://issues.apache.org/jira/browse/AURORA-1772
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is a refactor with addition of HttpClient injected into Webhook class as 
> opposed to previous usage of lower level HtttpURLConnection objects. 
> Additionally due to peformance issues, it is unncessary to POST entire task 
> state to webhook endpoint on every scheduler restart so that is removed in 
> this patch.
> 
> 
> Diffs
> -
> 
>   build.gradle d5a3a7a3bdb8349de6fc01d4a6271b32d942e531 
>   src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
> e54aa19d67278fcb5586f07c399f09062f845a18 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 
> e4193f7518e66d210a6d5aae22a9f04e2d4984b3 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookModule.java 
> eaa70533a64740c74cebf15739b7142e2d3a4d11 
>   src/main/resources/org/apache/aurora/scheduler/webhook.json 
> 00787985510d7d415b8074bef06d28b635c78b09 
>   src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java 
> 488eefd14c3e67a41a75c809397c8d19f83cc08a 
> 
> Diff: https://reviews.apache.org/r/51980/diff/
> 
> 
> Testing
> ---
> 
> Part of reason for refactor is to allow easier testing so cleaner unit tests 
> were added with more code coverage.
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>

Re: Review Request 51980: Refactor of Webhook and no longer posting entire task state via webhook on scheduler restart

2016-09-19 Thread Dmitriy Shirchenko 

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



@ReviewBot retry

- Dmitriy Shirchenko


On Sept. 19, 2016, 6:28 p.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51980/
> ---
> 
> (Updated Sept. 19, 2016, 6:28 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1772
> https://issues.apache.org/jira/browse/AURORA-1772
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is a refactor with addition of HttpClient injected into Webhook class as 
> opposed to previous usage of lower level HtttpURLConnection objects. 
> Additionally due to peformance issues, it is unncessary to POST entire task 
> state to webhook endpoint on every scheduler restart so that is removed in 
> this patch.
> 
> 
> Diffs
> -
> 
>   build.gradle d5a3a7a3bdb8349de6fc01d4a6271b32d942e531 
>   src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
> e54aa19d67278fcb5586f07c399f09062f845a18 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 
> e4193f7518e66d210a6d5aae22a9f04e2d4984b3 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookModule.java 
> eaa70533a64740c74cebf15739b7142e2d3a4d11 
>   src/main/resources/org/apache/aurora/scheduler/webhook.json 
> 00787985510d7d415b8074bef06d28b635c78b09 
>   src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java 
> 488eefd14c3e67a41a75c809397c8d19f83cc08a 
> 
> Diff: https://reviews.apache.org/r/51980/diff/
> 
> 
> Testing
> ---
> 
> Part of reason for refactor is to allow easier testing so cleaner unit tests 
> were added with more code coverage.
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>

Re: Review Request 52049: Clarifying documentation for new contributors by adding a step for them to ask for their JIRA id to get whitelisted

2016-09-19 Thread Jake Farrell 

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


Ship it!




Ship It!

- Jake Farrell


On Sept. 19, 2016, 6:42 p.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52049/
> ---
> 
> (Updated Sept. 19, 2016, 6:42 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Jake Farrell, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Due to spam bots, JIRA accounts need to be whitelisted before they can 
> self-assign tasks.
> 
> 
> Diffs
> -
> 
>   CONTRIBUTING.md 13ebbe4be281f869d276ebe97bfae75123899cfb 
> 
> Diff: https://reviews.apache.org/r/52049/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>

Re: Review Request 52051: Add a release note about full task filesystem isolation.

2016-09-19 Thread Zameer Manji 

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


Ship it!




Ship It!

- Zameer Manji


On Sept. 19, 2016, 12:04 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52051/
> ---
> 
> (Updated Sept. 19, 2016, 12:04 p.m.)
> 
> 
> Review request for Aurora and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add a release note about full task filesystem isolation.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 8e656ca7000f2e7baa3daa0e40f134c8295674e4 
> 
> Diff: https://reviews.apache.org/r/52051/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 51980: Refactor of Webhook and no longer posting entire task state via webhook on scheduler restart

2016-09-19 Thread Aurora ReviewBot 

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



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

 # Create file stdout for capturing output. We 
can't use StringIO mock
 # because TestProcess is running fork.
 with open(os.path.join(td, 'sys_stdout'), 
'w+') as stdout:
   with open(os.path.join(td, 'sys_stderr'), 
'w+') as stderr:
 with mutable_sys():
   sys.stdout, sys.stderr = stdout, 
stderr
 
   p = TestProcess('process', 'echo hello 
world; echo >&2 hello stderr', 0,
   taskpath, sandbox, 
logger_destination=LoggerDestination.BOTH)
   p.start()
   rc = 
wait_for_rc(taskpath.getpath('process_checkpoint'))
 
   assert rc == 0
   # Check log files were created in std 
path with correct content
 > assert_log_content(taskpath, 'stdout', 
'hello world\n')
 
 src/test/python/apache/thermos/core/test_process.py:487: 
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 
 taskpath = 
 log_name = 'stdout'
 expected_content = 'hello world\n'
 
 def assert_log_content(taskpath, log_name, 
expected_content):
   log = 
taskpath.with_filename(log_name).getpath('process_logdir')
   assert os.path.exists(log)
   with open(log, 'r') as fp:
 >   assert fp.read() == expected_content
 E   assert '' == 'hello world\n'
 E + hello world
 
 src/test/python/apache/thermos/core/test_process.py:313: 
AssertionError
  generated xml file: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/415337499eb72578eab327a6487c1f5c9452b3d6.xml
 
  1 failed, 710 passed, 6 skipped, 1 warnings in 
327.06 seconds 
 
FAILURE


19:09:25 06:10   [complete]
   FAILURE


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

- Aurora ReviewBot


On Sept. 19, 2016, 6:28 p.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51980/
> ---
> 
> (Updated Sept. 19, 2016, 6:28 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1772
> https://issues.apache.org/jira/browse/AURORA-1772
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is a refactor with addition of HttpClient injected into Webhook class as 
> opposed to previous usage of lower level HtttpURLConnection objects. 
> Additionally due to peformance issues, it is unncessary to POST entire task 
> state to webhook endpoint on every scheduler restart so that is removed in 
> this patch.
> 
> 
> Diffs
> -
> 
>   build.gradle d5a3a7a3bdb8349de6fc01d4a6271b32d942e531 
>   src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
> e54aa19d67278fcb5586f07c399f09062f845a18 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 
> e4193f7518e66d210a6d5aae22a9f04e2d4984b3 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookModule.java 
> eaa70533a64740c74cebf15739b7142e2d3a4d11 
>   src/main/resources/org/apache/aurora/scheduler/webhook.json 
> 00787985510d7d415b8074bef06d28b635c78b09 
>   src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java 
> 488eefd14c3e67a41a75c809397c8d19f83cc08a 
> 
> Diff: https://reviews.apache.org/r/51980/diff/
> 
> 
> Testing
> ---
> 
> Part of reason for refactor is to allow easier testing so cleaner unit tests 
> were added with more code coverage.
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>

Review Request 52051: Add a release note about full task filesystem isolation.

2016-09-19 Thread Joshua Cohen 

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

Review request for Aurora and Zameer Manji.


Repository: aurora


Description
---

Add a release note about full task filesystem isolation.


Diffs
-

  RELEASE-NOTES.md 8e656ca7000f2e7baa3daa0e40f134c8295674e4 

Diff: https://reviews.apache.org/r/52051/diff/


Testing
---


Thanks,

Joshua Cohen

Re: Review Request 52046: Adding gpg key for jco...@apache.org

2016-09-19 Thread Aurora ReviewBot 

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



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

 # Create file stdout for capturing output. We 
can't use StringIO mock
 # because TestProcess is running fork.
 with open(os.path.join(td, 'sys_stdout'), 
'w+') as stdout:
   with open(os.path.join(td, 'sys_stderr'), 
'w+') as stderr:
 with mutable_sys():
   sys.stdout, sys.stderr = stdout, 
stderr
 
   p = TestProcess('process', 'echo hello 
world; echo >&2 hello stderr', 0,
   taskpath, sandbox, 
logger_destination=LoggerDestination.BOTH)
   p.start()
   rc = 
wait_for_rc(taskpath.getpath('process_checkpoint'))
 
   assert rc == 0
   # Check log files were created in std 
path with correct content
 > assert_log_content(taskpath, 'stdout', 
'hello world\n')
 
 src/test/python/apache/thermos/core/test_process.py:487: 
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 
 taskpath = 
 log_name = 'stdout'
 expected_content = 'hello world\n'
 
 def assert_log_content(taskpath, log_name, 
expected_content):
   log = 
taskpath.with_filename(log_name).getpath('process_logdir')
   assert os.path.exists(log)
   with open(log, 'r') as fp:
 >   assert fp.read() == expected_content
 E   assert '' == 'hello world\n'
 E + hello world
 
 src/test/python/apache/thermos/core/test_process.py:313: 
AssertionError
  generated xml file: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/415337499eb72578eab327a6487c1f5c9452b3d6.xml
 
  1 failed, 710 passed, 6 skipped, 1 warnings in 
398.69 seconds 
 
FAILURE


18:54:17 07:27   [complete]
   FAILURE


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

- Aurora ReviewBot


On Sept. 19, 2016, 6:28 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52046/
> ---
> 
> (Updated Sept. 19, 2016, 6:28 p.m.)
> 
> 
> Review request for Aurora and Jake Farrell.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding gpg key for jco...@apache.org
> 
> 
> Diffs
> -
> 
>   KEYS d9e5ec932aea33dc5c2cb69e64bbefd1a155cced 
> 
> Diff: https://reviews.apache.org/r/52046/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 52046: Adding gpg key for jco...@apache.org

2016-09-19 Thread Jake Farrell 

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


Ship it!




Ship It!

- Jake Farrell


On Sept. 19, 2016, 6:28 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52046/
> ---
> 
> (Updated Sept. 19, 2016, 6:28 p.m.)
> 
> 
> Review request for Aurora and Jake Farrell.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding gpg key for jco...@apache.org
> 
> 
> Diffs
> -
> 
>   KEYS d9e5ec932aea33dc5c2cb69e64bbefd1a155cced 
> 
> Diff: https://reviews.apache.org/r/52046/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 52049: Clarifying documentation for new contributors by adding a step for them to ask for their JIRA id to get whitelisted

2016-09-19 Thread Dmitriy Shirchenko 

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

(Updated Sept. 19, 2016, 6:42 p.m.)


Review request for Aurora, Joshua Cohen, Jake Farrell, and Zameer Manji.


Repository: aurora


Description
---

Due to spam bots, JIRA accounts need to be whitelisted before they can 
self-assign tasks.


Diffs (updated)
-

  CONTRIBUTING.md 13ebbe4be281f869d276ebe97bfae75123899cfb 

Diff: https://reviews.apache.org/r/52049/diff/


Testing
---


Thanks,

Dmitriy Shirchenko

Re: Review Request 52049: Clarifying documentation for new contributors by adding a step for them to ask for their JIRA id to get whitelisted

2016-09-19 Thread Zameer Manji 

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


Ship it!




Ship It!

- Zameer Manji


On Sept. 19, 2016, 11:38 a.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52049/
> ---
> 
> (Updated Sept. 19, 2016, 11:38 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Jake Farrell, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Due to spam bots, JIRA accounts need to be whitelisted before they can 
> self-assign tasks.
> 
> 
> Diffs
> -
> 
>   CONTRIBUTING.md 13ebbe4be281f869d276ebe97bfae75123899cfb 
> 
> Diff: https://reviews.apache.org/r/52049/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>

Review Request 52049: Clarifying documentation for new contributors by adding a step for them to ask for their JIRA id to get whitelisted

2016-09-19 Thread Dmitriy Shirchenko 

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

Review request for Aurora, Joshua Cohen, Jake Farrell, and Zameer Manji.


Repository: aurora


Description
---

Due to spam bots, JIRA accounts need to be whitelisted before they can 
self-assign tasks.


Diffs
-

  CONTRIBUTING.md 13ebbe4be281f869d276ebe97bfae75123899cfb 

Diff: https://reviews.apache.org/r/52049/diff/


Testing
---


Thanks,

Dmitriy Shirchenko

Review Request 52046: Adding gpg key for jco...@apache.org

2016-09-19 Thread Joshua Cohen 

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

Review request for Aurora and Jake Farrell.


Repository: aurora


Description
---

Adding gpg key for jco...@apache.org


Diffs
-

  KEYS d9e5ec932aea33dc5c2cb69e64bbefd1a155cced 

Diff: https://reviews.apache.org/r/52046/diff/


Testing
---


Thanks,

Joshua Cohen

Re: Review Request 51980: Refactor of Webhook and no longer posting entire task state via webhook on scheduler restart

2016-09-19 Thread Dmitriy Shirchenko 

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

(Updated Sept. 19, 2016, 6:28 p.m.)


Review request for Aurora, Maxim Khutornenko, Stephan Erb, and Zameer Manji.


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


Repository: aurora


Description
---

This is a refactor with addition of HttpClient injected into Webhook class as 
opposed to previous usage of lower level HtttpURLConnection objects. 
Additionally due to peformance issues, it is unncessary to POST entire task 
state to webhook endpoint on every scheduler restart so that is removed in this 
patch.


Diffs (updated)
-

  build.gradle d5a3a7a3bdb8349de6fc01d4a6271b32d942e531 
  src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
e54aa19d67278fcb5586f07c399f09062f845a18 
  src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 
e4193f7518e66d210a6d5aae22a9f04e2d4984b3 
  src/main/java/org/apache/aurora/scheduler/events/WebhookModule.java 
eaa70533a64740c74cebf15739b7142e2d3a4d11 
  src/main/resources/org/apache/aurora/scheduler/webhook.json 
00787985510d7d415b8074bef06d28b635c78b09 
  src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java 
488eefd14c3e67a41a75c809397c8d19f83cc08a 

Diff: https://reviews.apache.org/r/51980/diff/


Testing
---

Part of reason for refactor is to allow easier testing so cleaner unit tests 
were added with more code coverage.


Thanks,

Dmitriy Shirchenko

Re: Review Request 51980: Refactor of Webhook and no longer posting entire task state via webhook on scheduler restart

2016-09-19 Thread Zameer Manji 

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




src/main/java/org/apache/aurora/scheduler/events/Webhook.java (line 71)


Please leave a comment here that if the old state is not present it's 
because the scheduler just started up and we don't want to resend those event .


- Zameer Manji


On Sept. 16, 2016, 6:40 p.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51980/
> ---
> 
> (Updated Sept. 16, 2016, 6:40 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1772
> https://issues.apache.org/jira/browse/AURORA-1772
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is a refactor with addition of HttpClient injected into Webhook class as 
> opposed to previous usage of lower level HtttpURLConnection objects. 
> Additionally due to peformance issues, it is unncessary to POST entire task 
> state to webhook endpoint on every scheduler restart so that is removed in 
> this patch.
> 
> 
> Diffs
> -
> 
>   build.gradle d5a3a7a3bdb8349de6fc01d4a6271b32d942e531 
>   src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
> e54aa19d67278fcb5586f07c399f09062f845a18 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 
> e4193f7518e66d210a6d5aae22a9f04e2d4984b3 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookModule.java 
> eaa70533a64740c74cebf15739b7142e2d3a4d11 
>   src/main/resources/org/apache/aurora/scheduler/webhook.json 
> 00787985510d7d415b8074bef06d28b635c78b09 
>   src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java 
> 488eefd14c3e67a41a75c809397c8d19f83cc08a 
> 
> Diff: https://reviews.apache.org/r/51980/diff/
> 
> 
> Testing
> ---
> 
> Part of reason for refactor is to allow easier testing so cleaner unit tests 
> were added with more code coverage.
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>

Re: Review Request 51993: Added the 'reason' to the /pendingTasks endpoint

2016-09-19 Thread Maxim Khutornenko 

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




src/main/java/org/apache/aurora/scheduler/scheduling/TaskGroups.java (line 138)


I don't think this is the right approach. `TaskGroups` has no business in 
accessing the `NearestFit` functionality. 

Please, consider extracting 
[this](https://github.com/apache/aurora/blob/795a2728c623c35bd509d582c24684a6921c74ad/src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java#L194-L208)
 and making it a public method of the `NearestFit` itself (e.g.: 
`getPendingReason(Query.Builder query)`. You can query that method and marry 
the results right within the `PendingTasks`.


- Maxim Khutornenko


On Sept. 17, 2016, 9:36 p.m., Pradyumna Kaushik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51993/
> ---
> 
> (Updated Sept. 17, 2016, 9:36 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Added the 'reason' to the /pendingTasks endpoint
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/PendingTasks.java 
> c80e0c8adf80e12082a6952ae79b7d9cc960c5b6 
>   src/main/java/org/apache/aurora/scheduler/scheduling/TaskGroup.java 
> 5d319557057e27fd5fc6d3e553e9ca9139399c50 
>   src/main/java/org/apache/aurora/scheduler/scheduling/TaskGroups.java 
> d390c07522d22e43d79ce4370985f3643ef021ca 
>   src/test/java/org/apache/aurora/scheduler/scheduling/TaskGroupsTest.java 
> 88729626de5fa87b45472792c59cc0ff1ade3e93 
> 
> Diff: https://reviews.apache.org/r/51993/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Pradyumna Kaushik
> 
>