Hello Michael, Nice to see how things have progressed!
I did a quick review of your code, and here are my notes: 1. Overall nice work! You say you have little experience with Java but overall I think you did a good job. 2. I noticed there are several empty files (they just have a license header), what is the purpose of this? Did you plan to add more configuration at a later time? 3. I would remove the plugin from the KAR packaging, as it is not started automatically. I think that we could add documentation to deploy it as we did for the Salesforces and MailChimp connectors here: http://unomi.apache.org/manual/latest/index.html#_connectors 4. Also, would you mind moving the project to extensions? Most of the new extensions are there. The plugins directory might be removed at some point. 5. I saw the change in the ProfileService, seems interesting but I wonder if we don't need a more global solution for all the exceptions in the REST services? What do you think? 6. For the tests that you have commented out, maybe we could reduce the thread count to 2000? Would that work better for you? Thanks for all the work! I hope my notes are constructive enough I want to encourage you to continue not the opposite :) Regards, Serge On Mon, Dec 10, 2018 at 9:14 PM Michael Ghen <m...@mikeghen.com> wrote: > > Ok, makes sense. > > Would anyone mind reviewing my webhook action? I haven't written too much > Java code so I'd appreciate any feedback. > > https://github.com/apache/incubator-unomi/compare/master...mikeghen:feature/webhook-action?expand=1 > > I create this rule like so with Python and I run a server on localhost that > prints out the POST data, seems to work OK: > > import requests > > r = requests.post('http://localhost:8181/cxs/rules/', > auth=('karaf','karaf'), > json ={ > "metadata": { > "id": "webhookExample", > "name": "WebhookExample", > "description": "Webhook Example" > }, > "condition": { > ... > }, > "actions": [ > { > "parameterValues": { > "webHookUrl": "http://localhost:5000/test";, > "webHookMethod": "POST", > "webHookData": "{\"var1\":\"xyz\",\"var2\":\"xyz\"}", > "webHookHeaders": "Content-Type:application/json" > }, > "type": "webHookAction" > } > ] > }) > print("Rule Response Code:", r) > print("Rule Response Content:", r.content) > > When I added the POST functionality, I may have broken the GET > functionality. I'm not sure the best way to set up a test for this action, > any recommendations? > > On Mon, Dec 10, 2018 at 4:14 AM Serge Huber <shu...@apache.org> wrote: > > > Hello Michael, > > > > I didn't envision a mapping type but I think it could be an > > interesting one to consider. I'd need to give it some thought though > > because I'm not sure if ElasticSearch can work with that in the > > queries. > > > > Regards, > > Serge... > > On Mon, Dec 10, 2018 at 4:59 AM Michael Ghen <m...@mikeghen.com> wrote: > > > > > > The suggestion for a token in the string and parsing it will work, was > > > thinking the same thing. Do you envision adding a mapping type to > > > parameters? I was kind of looking into where that would be implemented > > but > > > it seems non-trivial. For now, I'll roll with a token approach for > > headers. > > > > > > On Tue, Dec 4, 2018 at 11:04 AM Serge Huber <shu...@apache.org> wrote: > > > > > > > On Tue, Dec 4, 2018 at 3:01 PM Michael Ghen <m...@mikeghen.com> wrote: > > > > > BTW, the new unomi: commands are very helpful in 1.4 > > > > > > > > I'm glad you like them, I have developed most of them out of need, > > > > especially to diagnose some problems for example in definition > > > > registration, rule execution or simply event format checks. > > > > > > > > My favorites are : > > > > event-tail > > > > event-view > > > > rule-tail > > > > rule-view > > > > rule-watch > > > > > > > > what are yours ? :) > > > > > > > > cheers, > > > > Serge... > > > > > >
