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