You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Kai Huang <te...@hotmail.com> on 2017/06/09 05:13:18 UTC
Re: Review Request 59940: Add a whitelist for TaskStateChange events
in Webhook.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59940/
-----------------------------------------------------------
(Updated June 9, 2017, 5:13 a.m.)
Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, Stephan Erb, and Zameer Manji.
Changes
-------
Update the summary, no code change.
Summary (updated)
-----------------
Add a whitelist for TaskStateChange events in Webhook.
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/main/java/org/apache/aurora/scheduler/events/WebhookModule.java 1f10af71830386652d21961b733bd0927c5436a1
src/main/resources/org/apache/aurora/scheduler/webhook_whitelist.json PRE-CREATION
src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java e8335d9b78dbf30bf3ae08b6bdd02018cea76f6b
Diff: https://reviews.apache.org/r/59940/diff/1/
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:
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": ["LOST", "FAILED"]
}
```
Thanks,
Kai Huang
Re: Review Request 59940: Add a whitelist for TaskStateChange events
in Webhook.
Posted by Kai Huang <te...@hotmail.com>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/59940/#comment251468>
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.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
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.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59940/#review177830
-----------------------------------------------------------
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 14, 2017, 12:40 a.m., Kai Huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59940/
> -----------------------------------------------------------
>
> (Updated June 14, 2017, 12:40 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/5/
>
>
> 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.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59940/#review177843
-----------------------------------------------------------
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, 4:34 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, 4:34 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
> -----
>
> 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/
>
>
> 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.
Posted by Kai Huang <te...@hotmail.com>.
-----------------------------------------------------------
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.
Posted by Santhosh Kumar Shanmugham <sa...@gmail.com>.
-----------------------------------------------------------
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.
Posted by Kai Huang <te...@hotmail.com>.
-----------------------------------------------------------
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 59940: Add a whitelist for TaskStateChange events
in Webhook.
Posted by Kai Huang <te...@hotmail.com>.
> On June 13, 2017, 6:30 p.m., Santhosh Kumar Shanmugham wrote:
> > src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java
> > Lines 103 (patched)
> > <https://reviews.apache.org/r/59940/diff/2/?file=1749520#file1749520line103>
> >
> > private?
> >
> > It is not being used outside the class.
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?
> On June 13, 2017, 6:30 p.m., Santhosh Kumar Shanmugham wrote:
> > src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java
> > Lines 107-109 (patched)
> > <https://reviews.apache.org/r/59940/diff/2/?file=1749520#file1749520line107>
> >
> > Make this package-private and mark it as @VisibleForTesting.
addressed
> On June 13, 2017, 6:30 p.m., Santhosh Kumar Shanmugham wrote:
> > src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java
> > Lines 111 (patched)
> > <https://reviews.apache.org/r/59940/diff/2/?file=1749520#file1749520line111>
> >
> > package-private?
addressed
- Kai
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59940/#review177769
-----------------------------------------------------------
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.
Posted by Santhosh Kumar Shanmugham <sa...@gmail.com>.
> 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)
> > <https://reviews.apache.org/r/59940/diff/2/?file=1749520#file1749520line103>
> >
> > 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.
Posted by Santhosh Kumar Shanmugham <sa...@gmail.com>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/59940/#comment251480>
private?
It is not being used outside the class.
src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java
Lines 107-109 (patched)
<https://reviews.apache.org/r/59940/#comment251481>
Make this package-private and mark it as @VisibleForTesting.
src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java
Lines 111 (patched)
<https://reviews.apache.org/r/59940/#comment251483>
package-private?
src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java
Lines 54-79 (patched)
<https://reviews.apache.org/r/59940/#comment251486>
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.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
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.
Posted by Kai Huang <te...@hotmail.com>.
-----------------------------------------------------------
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 59940: Add a whitelist for TaskStateChange events
in Webhook.
Posted by David McLaughlin <da...@dmclaughlin.com>.
> On June 13, 2017, 2:24 p.m., David McLaughlin wrote:
> > src/main/java/org/apache/aurora/scheduler/events/Webhook.java
> > Lines 54 (patched)
> > <https://reviews.apache.org/r/59940/diff/2/?file=1749519#file1749519line54>
> >
> > 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.
Posted by Kai Huang <te...@hotmail.com>.
> On June 13, 2017, 2:24 p.m., David McLaughlin wrote:
> > src/main/java/org/apache/aurora/scheduler/events/Webhook.java
> > Lines 54 (patched)
> > <https://reviews.apache.org/r/59940/diff/2/?file=1749519#file1749519line54>
> >
> > 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.
Posted by David McLaughlin <da...@dmclaughlin.com>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/59940/#comment251453>
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.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59940/#review177683
-----------------------------------------------------------
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 12, 2017, 3: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, 3: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.
Posted by Kai Huang <te...@hotmail.com>.
-----------------------------------------------------------
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.
Changes
-------
Add a WebhookInfoBuilder to obtain the desired testability after adding the whitelist field into WebhookInfo.
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)
-----
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/
Changes: https://reviews.apache.org/r/59940/diff/1-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.
Posted by Kai Huang <te...@hotmail.com>.
> On June 9, 2017, 5:21 a.m., David McLaughlin wrote:
> > src/main/java/org/apache/aurora/scheduler/events/WebhookModule.java
> > Line 108 (original), 112-122 (patched)
> > <https://reviews.apache.org/r/59940/diff/1/?file=1746583#file1746583line112>
> >
> > This is a test fixture and doesn't belong in this module (or src/main in general). This also applies to the existing code you've emulated here.
> >
> > Suggest refactoring the module with a constructor that accepts a WebhookInfo in order to achieve the desired testability.
Yes, I agree that we should refactor the Webhook module. Keep adding test fixtures as resource files doesn't seem to be a clean solution.
- Kai
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59940/#review177440
-----------------------------------------------------------
On June 9, 2017, 5:13 a.m., Kai Huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59940/
> -----------------------------------------------------------
>
> (Updated June 9, 2017, 5:13 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
> -----
>
> src/main/java/org/apache/aurora/scheduler/events/Webhook.java 3868779986285ac302d028f8713f683192951b83
> src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 37c0d7944c7a37e1a5c65dafb290fcdd28765db3
> src/main/java/org/apache/aurora/scheduler/events/WebhookModule.java 1f10af71830386652d21961b733bd0927c5436a1
> src/main/resources/org/apache/aurora/scheduler/webhook_whitelist.json PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java e8335d9b78dbf30bf3ae08b6bdd02018cea76f6b
>
>
> Diff: https://reviews.apache.org/r/59940/diff/1/
>
>
> 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.
Posted by David McLaughlin <da...@dmclaughlin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59940/#review177440
-----------------------------------------------------------
Looks good, other than the issue below.
src/main/java/org/apache/aurora/scheduler/events/WebhookModule.java
Line 108 (original), 112-122 (patched)
<https://reviews.apache.org/r/59940/#comment250999>
This is a test fixture and doesn't belong in this module (or src/main in general). This also applies to the existing code you've emulated here.
Suggest refactoring the module with a constructor that accepts a WebhookInfo in order to achieve the desired testability.
- David McLaughlin
On June 9, 2017, 5:13 a.m., Kai Huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59940/
> -----------------------------------------------------------
>
> (Updated June 9, 2017, 5:13 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
> -----
>
> src/main/java/org/apache/aurora/scheduler/events/Webhook.java 3868779986285ac302d028f8713f683192951b83
> src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 37c0d7944c7a37e1a5c65dafb290fcdd28765db3
> src/main/java/org/apache/aurora/scheduler/events/WebhookModule.java 1f10af71830386652d21961b733bd0927c5436a1
> src/main/resources/org/apache/aurora/scheduler/webhook_whitelist.json PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java e8335d9b78dbf30bf3ae08b6bdd02018cea76f6b
>
>
> Diff: https://reviews.apache.org/r/59940/diff/1/
>
>
> 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.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59940/#review177448
-----------------------------------------------------------
Ship it!
Master (73234d0) 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 9, 2017, 5:13 a.m., Kai Huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59940/
> -----------------------------------------------------------
>
> (Updated June 9, 2017, 5:13 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
> -----
>
> src/main/java/org/apache/aurora/scheduler/events/Webhook.java 3868779986285ac302d028f8713f683192951b83
> src/main/java/org/apache/aurora/scheduler/events/WebhookInfo.java 37c0d7944c7a37e1a5c65dafb290fcdd28765db3
> src/main/java/org/apache/aurora/scheduler/events/WebhookModule.java 1f10af71830386652d21961b733bd0927c5436a1
> src/main/resources/org/apache/aurora/scheduler/webhook_whitelist.json PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java e8335d9b78dbf30bf3ae08b6bdd02018cea76f6b
>
>
> Diff: https://reviews.apache.org/r/59940/diff/1/
>
>
> 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
>
>