date:20170613

Re: Review Request 59940: Add a whitelist for TaskStateChange events in Webhook.

2017-06-13 Thread Kai Huang


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

(Updated June 13, 2017, 11:34 p.m.)


Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, Stephan 
Erb, and Zameer Manji.


Changes
---

Updated the RELEASE-NOTES for this feature.


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


Repository: aurora


Description
---

Add a whitelist for TaskStateChange events in Webhook.

Aurora Scheduler has a webhook module that watches all TaskStateChanges and 
send events to configured endpoint. This will flood the endpoint with a lot of 
noise if we only care about certain types of TaskStateChange event(e.g. task 
state change from RUNNING -> LOST). This CR allows user to provide a whitelist 
of TaskStateChange event types in their webhook configuration file, so that the 
webhook will only post these events to the configured endpoint.


Diffs (updated)
-

  RELEASE-NOTES.md c73643d669363a56e530c2d8c79d6ea06cc9415b 
  docs/features/webhooks.md a060975488a50ea34c79aebd66d056df8a3b437e 
  src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
3868779986285ac302d028f8713f683192951b83 
  src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 
37c0d7944c7a37e1a5c65dafb290fcdd28765db3 
  src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java 
e8335d9b78dbf30bf3ae08b6bdd02018cea76f6b 


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

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


Testing
---

./build-support/jenkins/build.sh

This change is backward compatible. By default, all TaskStateChange statuses 
will be matched and posted to the configured endpoint.

The user can also match all TaskStateChange statuses using a wildcard character 
"*" in webhook.json like below:
```
{
  "headers": {
"Content-Type": "application/vnd.kafka.json.v1+json",
"Producer-Type": "reliable"
  },
  "targetURL": "http://localhost:5000/;,
  "timeoutMsec": 50,
  "statuses": ["*"]
}
```

If they are only interested in TaskStateChange statuses: LOST, FAILED, they can 
provide them in the whitelist:
```
{
  "headers": {
"Content-Type": "application/vnd.kafka.json.v1+json",
"Producer-Type": "reliable"
  },
  "targetURL": "http://localhost:5000/;,
  "timeoutMsec": 50,
  "statuses": ["LOST", "FAILED"]
}
```


Thanks,

Kai Huang

Re: Review Request 59940: Add a whitelist for TaskStateChange events in Webhook.

2017-06-13 Thread Aurora ReviewBot


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


Ship it!




Master (cb86e83) 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 June 13, 2017, 9:40 p.m., Kai Huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59940/
> ---
> 
> (Updated June 13, 2017, 9:40 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, 
> Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1934
> https://issues.apache.org/jira/browse/AURORA-1934
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add a whitelist for TaskStateChange events in Webhook.
> 
> Aurora Scheduler has a webhook module that watches all TaskStateChanges and 
> send events to configured endpoint. This will flood the endpoint with a lot 
> of noise if we only care about certain types of TaskStateChange event(e.g. 
> task state change from RUNNING -> LOST). This CR allows user to provide a 
> whitelist of TaskStateChange event types in their webhook configuration file, 
> so that the webhook will only post these events to the configured endpoint.
> 
> 
> Diffs
> -
> 
>   docs/features/webhooks.md a060975488a50ea34c79aebd66d056df8a3b437e 
>   src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
> 3868779986285ac302d028f8713f683192951b83 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 
> 37c0d7944c7a37e1a5c65dafb290fcdd28765db3 
>   src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java 
> e8335d9b78dbf30bf3ae08b6bdd02018cea76f6b 
> 
> 
> Diff: https://reviews.apache.org/r/59940/diff/4/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> 
> This change is backward compatible. By default, all TaskStateChange statuses 
> will be matched and posted to the configured endpoint.
> 
> The user can also match all TaskStateChange statuses using a wildcard 
> character "*" in webhook.json like below:
> ```
> {
>   "headers": {
> "Content-Type": "application/vnd.kafka.json.v1+json",
> "Producer-Type": "reliable"
>   },
>   "targetURL": "http://localhost:5000/;,
>   "timeoutMsec": 50,
>   "statuses": ["*"]
> }
> ```
> 
> If they are only interested in TaskStateChange statuses: LOST, FAILED, they 
> can provide them in the whitelist:
> ```
> {
>   "headers": {
> "Content-Type": "application/vnd.kafka.json.v1+json",
> "Producer-Type": "reliable"
>   },
>   "targetURL": "http://localhost:5000/;,
>   "timeoutMsec": 50,
>   "statuses": ["LOST", "FAILED"]
> }
> ```
> 
> 
> Thanks,
> 
> Kai Huang
> 
>

Re: Review Request 59940: Add a whitelist for TaskStateChange events in Webhook.

2017-06-13 Thread Santhosh Kumar Shanmugham


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


Ship it!




Add an entry to the RELEASE-NOTES.

- Santhosh Kumar Shanmugham


On June 13, 2017, 2:40 p.m., Kai Huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59940/
> ---
> 
> (Updated June 13, 2017, 2:40 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, 
> Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1934
> https://issues.apache.org/jira/browse/AURORA-1934
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add a whitelist for TaskStateChange events in Webhook.
> 
> Aurora Scheduler has a webhook module that watches all TaskStateChanges and 
> send events to configured endpoint. This will flood the endpoint with a lot 
> of noise if we only care about certain types of TaskStateChange event(e.g. 
> task state change from RUNNING -> LOST). This CR allows user to provide a 
> whitelist of TaskStateChange event types in their webhook configuration file, 
> so that the webhook will only post these events to the configured endpoint.
> 
> 
> Diffs
> -
> 
>   docs/features/webhooks.md a060975488a50ea34c79aebd66d056df8a3b437e 
>   src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
> 3868779986285ac302d028f8713f683192951b83 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 
> 37c0d7944c7a37e1a5c65dafb290fcdd28765db3 
>   src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java 
> e8335d9b78dbf30bf3ae08b6bdd02018cea76f6b 
> 
> 
> Diff: https://reviews.apache.org/r/59940/diff/4/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> 
> This change is backward compatible. By default, all TaskStateChange statuses 
> will be matched and posted to the configured endpoint.
> 
> The user can also match all TaskStateChange statuses using a wildcard 
> character "*" in webhook.json like below:
> ```
> {
>   "headers": {
> "Content-Type": "application/vnd.kafka.json.v1+json",
> "Producer-Type": "reliable"
>   },
>   "targetURL": "http://localhost:5000/;,
>   "timeoutMsec": 50,
>   "statuses": ["*"]
> }
> ```
> 
> If they are only interested in TaskStateChange statuses: LOST, FAILED, they 
> can provide them in the whitelist:
> ```
> {
>   "headers": {
> "Content-Type": "application/vnd.kafka.json.v1+json",
> "Producer-Type": "reliable"
>   },
>   "targetURL": "http://localhost:5000/;,
>   "timeoutMsec": 50,
>   "statuses": ["LOST", "FAILED"]
> }
> ```
> 
> 
> Thanks,
> 
> Kai Huang
> 
>

Re: Review Request 59940: Add a whitelist for TaskStateChange events in Webhook.

2017-06-13 Thread Santhosh Kumar Shanmugham



> On June 13, 2017, 11:30 a.m., Santhosh Kumar Shanmugham wrote:
> > src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java
> > Lines 103 (patched)
> > 
> >
> > private?
> > 
> > It is not being used outside the class.
> 
> Kai Huang wrote:
> Making it private does not pass pmd styleCheck: Avoid instantiation 
> through private constructors from outside of the constructors class. We need 
> to make it package private instead?

package-private works fine.


- Santhosh Kumar


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


On June 13, 2017, 2:40 p.m., Kai Huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59940/
> ---
> 
> (Updated June 13, 2017, 2:40 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, 
> Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1934
> https://issues.apache.org/jira/browse/AURORA-1934
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add a whitelist for TaskStateChange events in Webhook.
> 
> Aurora Scheduler has a webhook module that watches all TaskStateChanges and 
> send events to configured endpoint. This will flood the endpoint with a lot 
> of noise if we only care about certain types of TaskStateChange event(e.g. 
> task state change from RUNNING -> LOST). This CR allows user to provide a 
> whitelist of TaskStateChange event types in their webhook configuration file, 
> so that the webhook will only post these events to the configured endpoint.
> 
> 
> Diffs
> -
> 
>   docs/features/webhooks.md a060975488a50ea34c79aebd66d056df8a3b437e 
>   src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
> 3868779986285ac302d028f8713f683192951b83 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 
> 37c0d7944c7a37e1a5c65dafb290fcdd28765db3 
>   src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java 
> e8335d9b78dbf30bf3ae08b6bdd02018cea76f6b 
> 
> 
> Diff: https://reviews.apache.org/r/59940/diff/4/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> 
> This change is backward compatible. By default, all TaskStateChange statuses 
> will be matched and posted to the configured endpoint.
> 
> The user can also match all TaskStateChange statuses using a wildcard 
> character "*" in webhook.json like below:
> ```
> {
>   "headers": {
> "Content-Type": "application/vnd.kafka.json.v1+json",
> "Producer-Type": "reliable"
>   },
>   "targetURL": "http://localhost:5000/;,
>   "timeoutMsec": 50,
>   "statuses": ["*"]
> }
> ```
> 
> If they are only interested in TaskStateChange statuses: LOST, FAILED, they 
> can provide them in the whitelist:
> ```
> {
>   "headers": {
> "Content-Type": "application/vnd.kafka.json.v1+json",
> "Producer-Type": "reliable"
>   },
>   "targetURL": "http://localhost:5000/;,
>   "timeoutMsec": 50,
>   "statuses": ["LOST", "FAILED"]
> }
> ```
> 
> 
> Thanks,
> 
> Kai Huang
> 
>

Re: Review Request 59940: Add a whitelist for TaskStateChange events in Webhook.

2017-06-13 Thread Kai Huang


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

(Updated June 13, 2017, 9:40 p.m.)


Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, Stephan 
Erb, and Zameer Manji.


Changes
---

Fixed variable accessibility and naming issues.


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


Repository: aurora


Description
---

Add a whitelist for TaskStateChange events in Webhook.

Aurora Scheduler has a webhook module that watches all TaskStateChanges and 
send events to configured endpoint. This will flood the endpoint with a lot of 
noise if we only care about certain types of TaskStateChange event(e.g. task 
state change from RUNNING -> LOST). This CR allows user to provide a whitelist 
of TaskStateChange event types in their webhook configuration file, so that the 
webhook will only post these events to the configured endpoint.


Diffs (updated)
-

  docs/features/webhooks.md a060975488a50ea34c79aebd66d056df8a3b437e 
  src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
3868779986285ac302d028f8713f683192951b83 
  src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 
37c0d7944c7a37e1a5c65dafb290fcdd28765db3 
  src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java 
e8335d9b78dbf30bf3ae08b6bdd02018cea76f6b 


Diff: https://reviews.apache.org/r/59940/diff/4/

Changes: https://reviews.apache.org/r/59940/diff/3-4/


Testing
---

./build-support/jenkins/build.sh

This change is backward compatible. By default, all TaskStateChange statuses 
will be matched and posted to the configured endpoint.

The user can also match all TaskStateChange statuses using a wildcard character 
"*" in webhook.json like below:
```
{
  "headers": {
"Content-Type": "application/vnd.kafka.json.v1+json",
"Producer-Type": "reliable"
  },
  "targetURL": "http://localhost:5000/;,
  "timeoutMsec": 50,
  "statuses": ["*"]
}
```

If they are only interested in TaskStateChange statuses: LOST, FAILED, they can 
provide them in the whitelist:
```
{
  "headers": {
"Content-Type": "application/vnd.kafka.json.v1+json",
"Producer-Type": "reliable"
  },
  "targetURL": "http://localhost:5000/;,
  "timeoutMsec": 50,
  "statuses": ["LOST", "FAILED"]
}
```


Thanks,

Kai Huang

Re: Review Request 57524: Support setting the rootfs on Mesos Containers.

2017-06-13 Thread Jason Lai



> On June 8, 2017, 4:07 p.m., Stephan Erb wrote:
> > Thanks for the nudge, sorry for the epic delay.
> > 
> > I agree that it is valuable to have a mode that runs Aurora containers 
> > similar to other Mesos containers (as least until we support PODs). I am 
> > therefore OK with the direction and goal of this patch. Turns out, I also 
> > have a potential usecase for this: Use the Mesos agent 
> > `--default_container_info` option to inject each task into a container 
> > without having to adjust all tasks on the Aurora side.
> > 
> > 
> > Looking at the patch though, I am confused if it works as is. Please 
> > correct me if I am wrong:
> > 
> > * 
> > https://github.com/apache/aurora/blob/c85bffdd6f68312261697eee868d57069adda434/src/main/python/apache/aurora/executor/common/sandbox.py#L84
> >  implies we will run Thermos using the `FileSystemImageSandbox` .
> > * The `FileSystemImageSandbox` will instruct Thermos to call the 
> > containerizer again, even though Thermos itself will already be running 
> > within a container. 
> > https://github.com/apache/aurora/blob/c85bffdd6f68312261697eee868d57069adda434/src/main/python/apache/aurora/executor/thermos_task_runner.py#L265
> > * The `FileSystemImageSandbox` is also re-mounting volumes and could thus 
> > lead to problems as those will already be mounted by the outside 
> > containerizer call. 
> > https://github.com/apache/aurora/blob/c85bffdd6f68312261697eee868d57069adda434/src/main/python/apache/aurora/executor/common/sandbox.py#L302
> 
> Jason Lai wrote:
> Hi Stephan! Thank you very much for the reply, particularly on pointing 
> out Thermos executor's use of 
> [`FileSystemImageSandbox`](https://github.com/apache/aurora/blob/c85bffdd6f68312261697eee868d57069adda434/src/main/python/apache/aurora/executor/common/sandbox.py#L84)
>  when running inside of a Mesos container. In this case, I believe we need 
> another patch to the executor in addition to this one, for surpressing the 
> use of `FileSystemImageSandbox` in favor of `DockerDirectorySandbox` when the 
> scheduler is configured to launch containers with their own root FS'es, in 
> order to make this patch fully working.
> 
> I'm thinking about adding an extra executor command line argument 
> (`--image-root-fs`, default to `False`) to Thermos executor, so that when 
> `True`, `DockerDirectorySandbox` will be used instead.
> 
> I also evaluated the impact of using `DockerDirectorySandbox` on health 
> checkers. It seems that with an instance with `#is_filesystem_image` as 
> `False`, the health checker will not be wrapped with `mesos-containerizer 
> launch` and will thus perform correctly. So this overall looks like a viable 
> approach. What do you think?

Please feel free to purpose a good name for the extra executor command line 
argument, if `--image-root-fs` doesn't ring a bell to you.

I was also thinking about introducing `--no-mesos-containerizer-path` to set 
`mesos_containerizer_path` option to `None`, as the main purpose of this option 
is intended for collaborating in the scenario of `FileSystemImageSandbox`.

Please let me know what you think. Thanks a lot!


- Jason


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


On March 13, 2017, 4:36 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57524/
> ---
> 
> (Updated March 13, 2017, 4:36 p.m.)
> 
> 
> Review request for Aurora, Santhosh Kumar Shanmugham and Stephan Erb.
> 
> 
> Bugs: AURORA-1903
> https://issues.apache.org/jira/browse/AURORA-1903
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The mesos unified containerizer does not support absolute container path 
> mounts if no rootfs is set. This allows operators to switch between our 
> current behaviour (mounting images as a volume) and setting the rootfs. See 
> AURORA-1903 for more detailed analysis.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> f0b148cd158d61cd89cc51dca9f3fa4c6feb1b49 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  4dac9757a65e144142d36ee921b85a02a5311fe5 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettings.java
>  5c987fd051728486172c8afd34219e86d56f00d5 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 0d639f66db456858278b0485c91c40975c3b45ac 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> e1cd81e6fbd98f23046e6e775be268be4310c62a 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
>

Re: Review Request 57524: Support setting the rootfs on Mesos Containers.

2017-06-13 Thread Jason Lai



> On June 8, 2017, 4:07 p.m., Stephan Erb wrote:
> > Thanks for the nudge, sorry for the epic delay.
> > 
> > I agree that it is valuable to have a mode that runs Aurora containers 
> > similar to other Mesos containers (as least until we support PODs). I am 
> > therefore OK with the direction and goal of this patch. Turns out, I also 
> > have a potential usecase for this: Use the Mesos agent 
> > `--default_container_info` option to inject each task into a container 
> > without having to adjust all tasks on the Aurora side.
> > 
> > 
> > Looking at the patch though, I am confused if it works as is. Please 
> > correct me if I am wrong:
> > 
> > * 
> > https://github.com/apache/aurora/blob/c85bffdd6f68312261697eee868d57069adda434/src/main/python/apache/aurora/executor/common/sandbox.py#L84
> >  implies we will run Thermos using the `FileSystemImageSandbox` .
> > * The `FileSystemImageSandbox` will instruct Thermos to call the 
> > containerizer again, even though Thermos itself will already be running 
> > within a container. 
> > https://github.com/apache/aurora/blob/c85bffdd6f68312261697eee868d57069adda434/src/main/python/apache/aurora/executor/thermos_task_runner.py#L265
> > * The `FileSystemImageSandbox` is also re-mounting volumes and could thus 
> > lead to problems as those will already be mounted by the outside 
> > containerizer call. 
> > https://github.com/apache/aurora/blob/c85bffdd6f68312261697eee868d57069adda434/src/main/python/apache/aurora/executor/common/sandbox.py#L302

Hi Stephan! Thank you very much for the reply, particularly on pointing out 
Thermos executor's use of 
[`FileSystemImageSandbox`](https://github.com/apache/aurora/blob/c85bffdd6f68312261697eee868d57069adda434/src/main/python/apache/aurora/executor/common/sandbox.py#L84)
 when running inside of a Mesos container. In this case, I believe we need 
another patch to the executor in addition to this one, for surpressing the use 
of `FileSystemImageSandbox` in favor of `DockerDirectorySandbox` when the 
scheduler is configured to launch containers with their own root FS'es, in 
order to make this patch fully working.

I'm thinking about adding an extra executor command line argument 
(`--image-root-fs`, default to `False`) to Thermos executor, so that when 
`True`, `DockerDirectorySandbox` will be used instead.

I also evaluated the impact of using `DockerDirectorySandbox` on health 
checkers. It seems that with an instance with `#is_filesystem_image` as 
`False`, the health checker will not be wrapped with `mesos-containerizer 
launch` and will thus perform correctly. So this overall looks like a viable 
approach. What do you think?


- Jason


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


On March 13, 2017, 4:36 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57524/
> ---
> 
> (Updated March 13, 2017, 4:36 p.m.)
> 
> 
> Review request for Aurora, Santhosh Kumar Shanmugham and Stephan Erb.
> 
> 
> Bugs: AURORA-1903
> https://issues.apache.org/jira/browse/AURORA-1903
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The mesos unified containerizer does not support absolute container path 
> mounts if no rootfs is set. This allows operators to switch between our 
> current behaviour (mounting images as a volume) and setting the rootfs. See 
> AURORA-1903 for more detailed analysis.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> f0b148cd158d61cd89cc51dca9f3fa4c6feb1b49 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  4dac9757a65e144142d36ee921b85a02a5311fe5 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettings.java
>  5c987fd051728486172c8afd34219e86d56f00d5 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 0d639f66db456858278b0485c91c40975c3b45ac 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> e1cd81e6fbd98f23046e6e775be268be4310c62a 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 93cc34cf8393f969087cd0fd6f577228c00170e9 
> 
> 
> Diff: https://reviews.apache.org/r/57524/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Re: Review Request 59940: Add a whitelist for TaskStateChange events in Webhook.

2017-06-13 Thread Aurora ReviewBot


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


Ship it!




Master (cb86e83) 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 June 13, 2017, 6:21 p.m., Kai Huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59940/
> ---
> 
> (Updated June 13, 2017, 6:21 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, 
> Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1934
> https://issues.apache.org/jira/browse/AURORA-1934
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add a whitelist for TaskStateChange events in Webhook.
> 
> Aurora Scheduler has a webhook module that watches all TaskStateChanges and 
> send events to configured endpoint. This will flood the endpoint with a lot 
> of noise if we only care about certain types of TaskStateChange event(e.g. 
> task state change from RUNNING -> LOST). This CR allows user to provide a 
> whitelist of TaskStateChange event types in their webhook configuration file, 
> so that the webhook will only post these events to the configured endpoint.
> 
> 
> Diffs
> -
> 
>   docs/features/webhooks.md a060975488a50ea34c79aebd66d056df8a3b437e 
>   src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
> 3868779986285ac302d028f8713f683192951b83 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 
> 37c0d7944c7a37e1a5c65dafb290fcdd28765db3 
>   src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java 
> e8335d9b78dbf30bf3ae08b6bdd02018cea76f6b 
> 
> 
> Diff: https://reviews.apache.org/r/59940/diff/3/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> 
> This change is backward compatible. By default, all TaskStateChange statuses 
> will be matched and posted to the configured endpoint.
> 
> The user can also match all TaskStateChange statuses using a wildcard 
> character "*" in webhook.json like below:
> ```
> {
>   "headers": {
> "Content-Type": "application/vnd.kafka.json.v1+json",
> "Producer-Type": "reliable"
>   },
>   "targetURL": "http://localhost:5000/;,
>   "timeoutMsec": 50,
>   "statuses": ["*"]
> }
> ```
> 
> If they are only interested in TaskStateChange statuses: LOST, FAILED, they 
> can provide them in the whitelist:
> ```
> {
>   "headers": {
> "Content-Type": "application/vnd.kafka.json.v1+json",
> "Producer-Type": "reliable"
>   },
>   "targetURL": "http://localhost:5000/;,
>   "timeoutMsec": 50,
>   "statuses": ["LOST", "FAILED"]
> }
> ```
> 
> 
> Thanks,
> 
> Kai Huang
> 
>

Re: Review Request 59940: Add a whitelist for TaskStateChange events in Webhook.

2017-06-13 Thread Santhosh Kumar Shanmugham


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



LGTM. Minor comments.


src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java
Lines 103 (patched)


private?

It is not being used outside the class.



src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java
Lines 107-109 (patched)


Make this package-private and mark it as @VisibleForTesting.



src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java
Lines 111 (patched)


package-private?



src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java
Lines 54-79 (patched)


Make these constants all CAPS. This convention is already broken in the 
test, but lets make it better.


- Santhosh Kumar Shanmugham


On June 13, 2017, 11:21 a.m., Kai Huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59940/
> ---
> 
> (Updated June 13, 2017, 11:21 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, 
> Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1934
> https://issues.apache.org/jira/browse/AURORA-1934
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add a whitelist for TaskStateChange events in Webhook.
> 
> Aurora Scheduler has a webhook module that watches all TaskStateChanges and 
> send events to configured endpoint. This will flood the endpoint with a lot 
> of noise if we only care about certain types of TaskStateChange event(e.g. 
> task state change from RUNNING -> LOST). This CR allows user to provide a 
> whitelist of TaskStateChange event types in their webhook configuration file, 
> so that the webhook will only post these events to the configured endpoint.
> 
> 
> Diffs
> -
> 
>   docs/features/webhooks.md a060975488a50ea34c79aebd66d056df8a3b437e 
>   src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
> 3868779986285ac302d028f8713f683192951b83 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 
> 37c0d7944c7a37e1a5c65dafb290fcdd28765db3 
>   src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java 
> e8335d9b78dbf30bf3ae08b6bdd02018cea76f6b 
> 
> 
> Diff: https://reviews.apache.org/r/59940/diff/3/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> 
> This change is backward compatible. By default, all TaskStateChange statuses 
> will be matched and posted to the configured endpoint.
> 
> The user can also match all TaskStateChange statuses using a wildcard 
> character "*" in webhook.json like below:
> ```
> {
>   "headers": {
> "Content-Type": "application/vnd.kafka.json.v1+json",
> "Producer-Type": "reliable"
>   },
>   "targetURL": "http://localhost:5000/;,
>   "timeoutMsec": 50,
>   "statuses": ["*"]
> }
> ```
> 
> If they are only interested in TaskStateChange statuses: LOST, FAILED, they 
> can provide them in the whitelist:
> ```
> {
>   "headers": {
> "Content-Type": "application/vnd.kafka.json.v1+json",
> "Producer-Type": "reliable"
>   },
>   "targetURL": "http://localhost:5000/;,
>   "timeoutMsec": 50,
>   "statuses": ["LOST", "FAILED"]
> }
> ```
> 
> 
> Thanks,
> 
> Kai Huang
> 
>

Re: Review Request 59940: Add a whitelist for TaskStateChange events in Webhook.

2017-06-13 Thread Kai Huang


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

(Updated June 13, 2017, 6:21 p.m.)


Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, Stephan 
Erb, and Zameer Manji.


Changes
---

Update documentation for webhook feature.


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


Repository: aurora


Description
---

Add a whitelist for TaskStateChange events in Webhook.

Aurora Scheduler has a webhook module that watches all TaskStateChanges and 
send events to configured endpoint. This will flood the endpoint with a lot of 
noise if we only care about certain types of TaskStateChange event(e.g. task 
state change from RUNNING -> LOST). This CR allows user to provide a whitelist 
of TaskStateChange event types in their webhook configuration file, so that the 
webhook will only post these events to the configured endpoint.


Diffs (updated)
-

  docs/features/webhooks.md a060975488a50ea34c79aebd66d056df8a3b437e 
  src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
3868779986285ac302d028f8713f683192951b83 
  src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 
37c0d7944c7a37e1a5c65dafb290fcdd28765db3 
  src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java 
e8335d9b78dbf30bf3ae08b6bdd02018cea76f6b 


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

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


Testing
---

./build-support/jenkins/build.sh

This change is backward compatible. By default, all TaskStateChange statuses 
will be matched and posted to the configured endpoint.

The user can also match all TaskStateChange statuses using a wildcard character 
"*" in webhook.json like below:
```
{
  "headers": {
"Content-Type": "application/vnd.kafka.json.v1+json",
"Producer-Type": "reliable"
  },
  "targetURL": "http://localhost:5000/;,
  "timeoutMsec": 50,
  "statuses": ["*"]
}
```

If they are only interested in TaskStateChange statuses: LOST, FAILED, they can 
provide them in the whitelist:
```
{
  "headers": {
"Content-Type": "application/vnd.kafka.json.v1+json",
"Producer-Type": "reliable"
  },
  "targetURL": "http://localhost:5000/;,
  "timeoutMsec": 50,
  "statuses": ["LOST", "FAILED"]
}
```


Thanks,

Kai Huang

Re: Review Request 59733: Adding Configurable Wait Period for Graceful Shutdowns

2017-06-13 Thread Aurora ReviewBot


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


Ship it!




Master (40d9d4d) 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 June 13, 2017, 5:58 p.m., Jordan Ly wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59733/
> ---
> 
> (Updated June 13, 2017, 5:58 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, 
> Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1931
> https://issues.apache.org/jira/browse/AURORA-1931
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> We have some services that require more than the current 10 seconds given to 
> gracefully shutdown (they need to close resources, finish requests, etc).
> 
> We would like to be able to configure the amount of time we wait between each
> stage of the graceful shutdown sequence. See this 
> [proposal](https://docs.google.com/document/d/1Sl-KWNyt1j0nIndinqfJsH3pkUY5IYXfGWyLHU2wacs/edit?usp=sharing)
>  for a more in-depth
> analysis.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md d14c4ad2a1bbdfdc845f1ec6dcd931c16b94569a 
>   docs/reference/configuration.md 0040de17a85f5820c069474716678c66afce373b 
>   docs/reference/task-lifecycle.md cf1b6794dca92756cee4a54f00f33fd3b1a481ea 
>   src/main/python/apache/aurora/config/schema/base.py 
> b2692a648645a195a24491e4978fb833c6c20be8 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> 81461cb49ac223f3bdfa59e8c59e150a07771dea 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> c6c08983fb2d204afedd419798135d667169bce4 
>   src/main/python/apache/aurora/executor/http_lifecycle.py 
> 9280bf29da9bda1691adbf3a4c34c4f3d4900517 
>   src/test/python/apache/aurora/client/cli/test_inspect.py 
> 4a23c5984c2d093e2f53e93aec71418f84b65928 
>   
> src/test/python/apache/aurora/executor/bin/test_thermos_executor_entry_point.py
>  38deae69dce7d88dab87f70e0128e0b86091fdef 
>   src/test/python/apache/aurora/executor/test_http_lifecycle.py 
> a967e3410a4d2dc2e1721f505a4d76da9209d177 
>   src/test/python/apache/aurora/executor/test_thermos_executor.py 
> e628ccd21f0588d0c76001b8f336ffa844c3ec35 
>   src/test/python/apache/aurora/executor/test_thermos_task_runner.py 
> 1b92667bceabc8ea1540122477a51cb58ea2ae36 
> 
> 
> Diff: https://reviews.apache.org/r/59733/diff/6/
> 
> 
> Testing
> ---
> 
> Ran unit and integration tests.
> 
> Created and killed jobs with varying wait_escalation_secs values on the 
> Vagrant devcluster.
> 
> 
> Thanks,
> 
> Jordan Ly
> 
>

Re: Review Request 59733: Adding Configurable Wait Period for Graceful Shutdowns

2017-06-13 Thread Jordan Ly


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

(Updated June 13, 2017, 5:58 p.m.)


Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, Stephan 
Erb, and Zameer Manji.


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


Repository: aurora


Description
---

We have some services that require more than the current 10 seconds given to 
gracefully shutdown (they need to close resources, finish requests, etc).

We would like to be able to configure the amount of time we wait between each
stage of the graceful shutdown sequence. See this 
[proposal](https://docs.google.com/document/d/1Sl-KWNyt1j0nIndinqfJsH3pkUY5IYXfGWyLHU2wacs/edit?usp=sharing)
 for a more in-depth
analysis.


Diffs (updated)
-

  RELEASE-NOTES.md d14c4ad2a1bbdfdc845f1ec6dcd931c16b94569a 
  docs/reference/configuration.md 0040de17a85f5820c069474716678c66afce373b 
  docs/reference/task-lifecycle.md cf1b6794dca92756cee4a54f00f33fd3b1a481ea 
  src/main/python/apache/aurora/config/schema/base.py 
b2692a648645a195a24491e4978fb833c6c20be8 
  src/main/python/apache/aurora/executor/aurora_executor.py 
81461cb49ac223f3bdfa59e8c59e150a07771dea 
  src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
c6c08983fb2d204afedd419798135d667169bce4 
  src/main/python/apache/aurora/executor/http_lifecycle.py 
9280bf29da9bda1691adbf3a4c34c4f3d4900517 
  src/test/python/apache/aurora/client/cli/test_inspect.py 
4a23c5984c2d093e2f53e93aec71418f84b65928 
  
src/test/python/apache/aurora/executor/bin/test_thermos_executor_entry_point.py 
38deae69dce7d88dab87f70e0128e0b86091fdef 
  src/test/python/apache/aurora/executor/test_http_lifecycle.py 
a967e3410a4d2dc2e1721f505a4d76da9209d177 
  src/test/python/apache/aurora/executor/test_thermos_executor.py 
e628ccd21f0588d0c76001b8f336ffa844c3ec35 
  src/test/python/apache/aurora/executor/test_thermos_task_runner.py 
1b92667bceabc8ea1540122477a51cb58ea2ae36 


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

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


Testing
---

Ran unit and integration tests.

Created and killed jobs with varying wait_escalation_secs values on the Vagrant 
devcluster.


Thanks,

Jordan Ly

Re: Review Request 59940: Add a whitelist for TaskStateChange events in Webhook.

2017-06-13 Thread David McLaughlin



> On June 13, 2017, 2:24 p.m., David McLaughlin wrote:
> > src/main/java/org/apache/aurora/scheduler/events/Webhook.java
> > Lines 54 (patched)
> > 
> >
> > This seems clearer:
> > 
> >   webhookInfo.getWhitelistedStatuses().isPresent() && 
> > webhookInfo.getWhitelistedStatuses().get().contains(status)
> 
> Kai Huang wrote:
> The negation here corresponds to case a): the whitelist is absent (not 
> provided by user or contains wildcard character)

Ah, I misread the logic. Shipit!


- David


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


On June 12, 2017, 10:51 p.m., Kai Huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59940/
> ---
> 
> (Updated June 12, 2017, 10:51 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, 
> Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1934
> https://issues.apache.org/jira/browse/AURORA-1934
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add a whitelist for TaskStateChange events in Webhook.
> 
> Aurora Scheduler has a webhook module that watches all TaskStateChanges and 
> send events to configured endpoint. This will flood the endpoint with a lot 
> of noise if we only care about certain types of TaskStateChange event(e.g. 
> task state change from RUNNING -> LOST). This CR allows user to provide a 
> whitelist of TaskStateChange event types in their webhook configuration file, 
> so that the webhook will only post these events to the configured endpoint.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
> 3868779986285ac302d028f8713f683192951b83 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 
> 37c0d7944c7a37e1a5c65dafb290fcdd28765db3 
>   src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java 
> e8335d9b78dbf30bf3ae08b6bdd02018cea76f6b 
> 
> 
> Diff: https://reviews.apache.org/r/59940/diff/2/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> 
> This change is backward compatible. By default, all TaskStateChange statuses 
> will be matched and posted to the configured endpoint.
> 
> The user can also match all TaskStateChange statuses using a wildcard 
> character "*" in webhook.json like below:
> ```
> {
>   "headers": {
> "Content-Type": "application/vnd.kafka.json.v1+json",
> "Producer-Type": "reliable"
>   },
>   "targetURL": "http://localhost:5000/;,
>   "timeoutMsec": 50,
>   "statuses": ["*"]
> }
> ```
> 
> If they are only interested in TaskStateChange statuses: LOST, FAILED, they 
> can provide them in the whitelist:
> ```
> {
>   "headers": {
> "Content-Type": "application/vnd.kafka.json.v1+json",
> "Producer-Type": "reliable"
>   },
>   "targetURL": "http://localhost:5000/;,
>   "timeoutMsec": 50,
>   "statuses": ["LOST", "FAILED"]
> }
> ```
> 
> 
> Thanks,
> 
> Kai Huang
> 
>

Re: Review Request 59940: Add a whitelist for TaskStateChange events in Webhook.

2017-06-13 Thread Kai Huang



> On June 13, 2017, 2:24 p.m., David McLaughlin wrote:
> > src/main/java/org/apache/aurora/scheduler/events/Webhook.java
> > Lines 54 (patched)
> > 
> >
> > This seems clearer:
> > 
> >   webhookInfo.getWhitelistedStatuses().isPresent() && 
> > webhookInfo.getWhitelistedStatuses().get().contains(status)

The negation here corresponds to case a): the whitelist is absent (not provided 
by user or contains wildcard character)


- Kai


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


On June 12, 2017, 10:51 p.m., Kai Huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59940/
> ---
> 
> (Updated June 12, 2017, 10:51 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, 
> Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1934
> https://issues.apache.org/jira/browse/AURORA-1934
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add a whitelist for TaskStateChange events in Webhook.
> 
> Aurora Scheduler has a webhook module that watches all TaskStateChanges and 
> send events to configured endpoint. This will flood the endpoint with a lot 
> of noise if we only care about certain types of TaskStateChange event(e.g. 
> task state change from RUNNING -> LOST). This CR allows user to provide a 
> whitelist of TaskStateChange event types in their webhook configuration file, 
> so that the webhook will only post these events to the configured endpoint.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
> 3868779986285ac302d028f8713f683192951b83 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 
> 37c0d7944c7a37e1a5c65dafb290fcdd28765db3 
>   src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java 
> e8335d9b78dbf30bf3ae08b6bdd02018cea76f6b 
> 
> 
> Diff: https://reviews.apache.org/r/59940/diff/2/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> 
> This change is backward compatible. By default, all TaskStateChange statuses 
> will be matched and posted to the configured endpoint.
> 
> The user can also match all TaskStateChange statuses using a wildcard 
> character "*" in webhook.json like below:
> ```
> {
>   "headers": {
> "Content-Type": "application/vnd.kafka.json.v1+json",
> "Producer-Type": "reliable"
>   },
>   "targetURL": "http://localhost:5000/;,
>   "timeoutMsec": 50,
>   "statuses": ["*"]
> }
> ```
> 
> If they are only interested in TaskStateChange statuses: LOST, FAILED, they 
> can provide them in the whitelist:
> ```
> {
>   "headers": {
> "Content-Type": "application/vnd.kafka.json.v1+json",
> "Producer-Type": "reliable"
>   },
>   "targetURL": "http://localhost:5000/;,
>   "timeoutMsec": 50,
>   "statuses": ["LOST", "FAILED"]
> }
> ```
> 
> 
> Thanks,
> 
> Kai Huang
> 
>

Re: Review Request 59940: Add a whitelist for TaskStateChange events in Webhook.

2017-06-13 Thread Kai Huang


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




src/main/java/org/apache/aurora/scheduler/events/Webhook.java
Lines 54 (patched)


So a task status is whitelisted if:

a) the whitelist is absent(not provided by user or contains wildcard 
character)

b) the task status is explicitly specified in the whitelist.


- Kai Huang


On June 12, 2017, 10:51 p.m., Kai Huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59940/
> ---
> 
> (Updated June 12, 2017, 10:51 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, 
> Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1934
> https://issues.apache.org/jira/browse/AURORA-1934
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add a whitelist for TaskStateChange events in Webhook.
> 
> Aurora Scheduler has a webhook module that watches all TaskStateChanges and 
> send events to configured endpoint. This will flood the endpoint with a lot 
> of noise if we only care about certain types of TaskStateChange event(e.g. 
> task state change from RUNNING -> LOST). This CR allows user to provide a 
> whitelist of TaskStateChange event types in their webhook configuration file, 
> so that the webhook will only post these events to the configured endpoint.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
> 3868779986285ac302d028f8713f683192951b83 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 
> 37c0d7944c7a37e1a5c65dafb290fcdd28765db3 
>   src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java 
> e8335d9b78dbf30bf3ae08b6bdd02018cea76f6b 
> 
> 
> Diff: https://reviews.apache.org/r/59940/diff/2/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> 
> This change is backward compatible. By default, all TaskStateChange statuses 
> will be matched and posted to the configured endpoint.
> 
> The user can also match all TaskStateChange statuses using a wildcard 
> character "*" in webhook.json like below:
> ```
> {
>   "headers": {
> "Content-Type": "application/vnd.kafka.json.v1+json",
> "Producer-Type": "reliable"
>   },
>   "targetURL": "http://localhost:5000/;,
>   "timeoutMsec": 50,
>   "statuses": ["*"]
> }
> ```
> 
> If they are only interested in TaskStateChange statuses: LOST, FAILED, they 
> can provide them in the whitelist:
> ```
> {
>   "headers": {
> "Content-Type": "application/vnd.kafka.json.v1+json",
> "Producer-Type": "reliable"
>   },
>   "targetURL": "http://localhost:5000/;,
>   "timeoutMsec": 50,
>   "statuses": ["LOST", "FAILED"]
> }
> ```
> 
> 
> Thanks,
> 
> Kai Huang
> 
>

Re: Review Request 59940: Add a whitelist for TaskStateChange events in Webhook.

2017-06-13 Thread David McLaughlin


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


Ship it!




Just a small nit, otherwise LGTM.


src/main/java/org/apache/aurora/scheduler/events/Webhook.java
Lines 54 (patched)


This seems clearer:

  webhookInfo.getWhitelistedStatuses().isPresent() && 
webhookInfo.getWhitelistedStatuses().get().contains(status)


- David McLaughlin


On June 12, 2017, 10:51 p.m., Kai Huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59940/
> ---
> 
> (Updated June 12, 2017, 10:51 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, 
> Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1934
> https://issues.apache.org/jira/browse/AURORA-1934
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add a whitelist for TaskStateChange events in Webhook.
> 
> Aurora Scheduler has a webhook module that watches all TaskStateChanges and 
> send events to configured endpoint. This will flood the endpoint with a lot 
> of noise if we only care about certain types of TaskStateChange event(e.g. 
> task state change from RUNNING -> LOST). This CR allows user to provide a 
> whitelist of TaskStateChange event types in their webhook configuration file, 
> so that the webhook will only post these events to the configured endpoint.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
> 3868779986285ac302d028f8713f683192951b83 
>   src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 
> 37c0d7944c7a37e1a5c65dafb290fcdd28765db3 
>   src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java 
> e8335d9b78dbf30bf3ae08b6bdd02018cea76f6b 
> 
> 
> Diff: https://reviews.apache.org/r/59940/diff/2/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> 
> This change is backward compatible. By default, all TaskStateChange statuses 
> will be matched and posted to the configured endpoint.
> 
> The user can also match all TaskStateChange statuses using a wildcard 
> character "*" in webhook.json like below:
> ```
> {
>   "headers": {
> "Content-Type": "application/vnd.kafka.json.v1+json",
> "Producer-Type": "reliable"
>   },
>   "targetURL": "http://localhost:5000/;,
>   "timeoutMsec": 50,
>   "statuses": ["*"]
> }
> ```
> 
> If they are only interested in TaskStateChange statuses: LOST, FAILED, they 
> can provide them in the whitelist:
> ```
> {
>   "headers": {
> "Content-Type": "application/vnd.kafka.json.v1+json",
> "Producer-Type": "reliable"
>   },
>   "targetURL": "http://localhost:5000/;,
>   "timeoutMsec": 50,
>   "statuses": ["LOST", "FAILED"]
> }
> ```
> 
> 
> Thanks,
> 
> Kai Huang
> 
>

Re: Review Request 59940: Add a whitelist for TaskStateChange events in Webhook.

Re: Review Request 59940: Add a whitelist for TaskStateChange events in Webhook.

Re: Review Request 59940: Add a whitelist for TaskStateChange events in Webhook.

Re: Review Request 59940: Add a whitelist for TaskStateChange events in Webhook.

Re: Review Request 59940: Add a whitelist for TaskStateChange events in Webhook.

Re: Review Request 57524: Support setting the rootfs on Mesos Containers.

Re: Review Request 57524: Support setting the rootfs on Mesos Containers.

Re: Review Request 59940: Add a whitelist for TaskStateChange events in Webhook.

Re: Review Request 59940: Add a whitelist for TaskStateChange events in Webhook.

Re: Review Request 59940: Add a whitelist for TaskStateChange events in Webhook.

Re: Review Request 59733: Adding Configurable Wait Period for Graceful Shutdowns

Re: Review Request 59733: Adding Configurable Wait Period for Graceful Shutdowns

Re: Review Request 59940: Add a whitelist for TaskStateChange events in Webhook.

Re: Review Request 59940: Add a whitelist for TaskStateChange events in Webhook.

Re: Review Request 59940: Add a whitelist for TaskStateChange events in Webhook.

Re: Review Request 59940: Add a whitelist for TaskStateChange events in Webhook.

16 matches

Site Navigation

Mail list logo

Footer information