Thanks, really appreciated being a module I am not using much, so I haven't approach it from KEMI perspective yet!

@lazedo - can you quickly check this PR and see if there is something you want to comment on?

Otherwise, I am fine to merge.

@lazedo requested changes on this pull request.

thank you for your contribution. some required changes

• if(faked_msg_init()<0)

+ { + LM_ERR("failed to init kazoo local sip msg\n"); + return -1; + } +

why was this added ?

• sr_kemi_eng_t *keng = NULL;

+ if(kazoo_event_callback.s!=NULL && kazoo_event_callback.len>0) { + keng = sr_kemi_eng_get(); + if(keng==NULL) { + LM_ERR("failed to find kemi engine\n"); + return -1; + } + kazoo_route_no=-1; + } + else { + route_no=route_lookup(&event_rt, "kazoo:consumer-event"); + if (route_no==-1) + { + LM_ERR("failed to find event_route[kazoo:consumer-event]\n"); + return -1; + } + if (event_rt.rlist[route_no]==0) + { + LM_WARN("event_route[kazoo:consumer-event] is empty\n"); + } + kazoo_route_no=route_no; + } + +

please use a different logic here. `kazoo:consumer-event` is not mandatory. you could set a variable like `kz_use_kemi` that is set if `kazoo_event_callback` is set and `kemi` is valid. then use that variable when needed

• if (kazoo_route_no>=0) {

+ rt = route_get(&event_rt, "kazoo:mod-init"); + if(rt>=0 && event_rt.rlist[rt]!=NULL) { + LM_DBG("executing event_route[kazoo:mod-init] (%d)\n", rt);

formatting looks odd

•  if (kazoo_route_no>=0) {
  

sprintf(buffer, "kazoo:consumer-event-%.*s-%.*s",ev_category.len, ev_category.s, ev_name.len, ev_name.s); - for (p=buffer ; *p; ++p) *p = tolower(*p); - for (p=buffer ; *p; ++p) if(*p == '_') *p = '-'; - if(kz_amqp_consumer_fire_event(buffer) != 0) { - sprintf(buffer, "kazoo:consumer-event-%.*s",ev_category.len, ev_category.s); for (p=buffer ; *p; ++p) *p = tolower(*p); for (p=buffer ; *p; ++p) if(*p == '_') *p = '-'; if(kz_amqp_consumer_fire_event(buffer) != 0) { - sprintf(buffer, "kazoo:consumer-event-%s-%s", key, subkey); + sprintf(buffer, "kazoo:consumer-event-%.*s",ev_category.len, ev_category.s); for (p=buffer ; *p; ++p) *p = tolower(*p); for (p=buffer ; *p; ++p) if(*p == '_') *p = '-'; if(kz_amqp_consumer_fire_event(buffer) != 0) { - sprintf(buffer, "kazoo:consumer-event-%s", key); + sprintf(buffer, "kazoo:consumer-event-%s-%s", key, subkey); for (p=buffer ; *p; ++p) *p = tolower(*p); for (p=buffer ; *p; ++p) if(*p == '_') *p = '-'; - if(kz_amqp_consumer_fire_event(buffer) != 0) { - sprintf(buffer, "kazoo:consumer-event"); - if(kz_amqp_consumer_fire_event(buffer) != 0) { - LM_ERR("kazoo:consumer-event not found\n"); - } - } + if(kz_amqp_consumer_fire_event(buffer) != 0) { + sprintf(buffer, "kazoo:consumer-event-%s", key); + for (p=buffer ; *p; ++p) *p = tolower(*p); + for (p=buffer ; *p; ++p) if(*p == '_') *p = '-'; + if(kz_amqp_consumer_fire_event(buffer) != 0) { + sprintf(buffer, "kazoo:consumer-event"); + if(kz_amqp_consumer_fire_event(buffer) != 0) { + LM_ERR("kazoo:consumer-event not found\n"); + } + } + } } } + } else { + keng = sr_kemi_eng_get(); + if(keng!=NULL) { + sip_msg_t *msg; + str evrtname = str_init("kazoo:consumer-event"); + + rtb = get_route_type(); + msg = faked_msg_next(); + if(sr_kemi_route(keng, msg, EVENT_ROUTE, &kazoo_event_callback, &evrtname)<0) { + LM_ERR("error running event route kemi callback\n"); + } + set_route_type(rtb); + } else { + LM_ERR("no event route or kemi callback found for execution\n"); + } } +

split this in two different functions that are evoked depending on `kemi` being active

• rt = route_get(&event_rt, "kazoo:mod-init");

- if(rt>=0 && event_rt.rlist[rt]!=NULL) { - LM_DBG("executing event_route[kazoo:mod-init] (%d)\n", rt); - if(faked_msg_init()<0) - return -1; - fmsg = faked_msg_next(); - rtb = get_route_type(); - set_route_type(REQUEST_ROUTE); - init_run_actions_ctx(&ctx); - run_top_route(event_rt.rlist[rt], fmsg, &ctx); - if(ctx.run_flags&DROP_R_F) - { - LM_ERR("exit due to 'drop' in event route\n"); - return -1; + if (kazoo_route_no>=0) { + rt = route_get(&event_rt, "kazoo:mod-init"); + if(rt>=0 && event_rt.rlist[rt]!=NULL) { + LM_DBG("executing event_route[kazoo:mod-init] (%d)\n", rt); + if(faked_msg_init()<0) + return -1; + fmsg = faked_msg_next(); + rtb = get_route_type(); + set_route_type(REQUEST_ROUTE); + init_run_actions_ctx(&ctx); + run_top_route(event_rt.rlist[rt], fmsg, &ctx); + if(ctx.run_flags&DROP_R_F) + { + LM_ERR("exit due to 'drop' in event route\n"); + return -1; + } + set_route_type(rtb); + } + } + else { + keng = sr_kemi_eng_get(); + if(keng!=NULL) { + sip_msg_t *msg; + str evrtname = str_init("kazoo:mod-init"); + rtb = get_route_type(); + msg = faked_msg_next(); + if(sr_kemi_route(keng, msg, EVENT_ROUTE, &kazoo_event_callback, &evrtname)<0) { + LM_ERR("error running event route kemi callback\n"); + } + set_route_type(rtb); + } + else { + LM_ERR("no event route or kemi callback found for execution\n");

split this in two different function that are evoked by `fire_init_event` depending on `kemi` being active

I have addressed the suggested changes in the new commit: - removed route_lookup for kazoo:consumer-event as it may or may not exist - depending on using cfg or kemi: - fire_init_event() calls fire_init_event_cfg() or fire_init_event_kemi() - kz_amqp_consumer_event() calls kz_amqp_consumer_event_cfg() or and tidied up formatting.

Tested on my test servers for: kazoo_subscribe, kazoo_publish, kazoo:consumer-event, with the traditional cfg and Kemi (Lua). kz_amqp_consumer_event_kemi()

@tao-communications commented on this pull request.

• sr_kemi_eng_t *keng = NULL;

+ if(kazoo_event_callback.s!=NULL && kazoo_event_callback.len>0) { + keng = sr_kemi_eng_get(); + if(keng==NULL) { + LM_ERR("failed to find kemi engine\n"); + return -1; + } + kazoo_route_no=-1; + } + else { + route_no=route_lookup(&event_rt, "kazoo:consumer-event"); + if (route_no==-1) + { + LM_ERR("failed to find event_route[kazoo:consumer-event]\n"); + return -1; + } + if (event_rt.rlist[route_no]==0) + { + LM_WARN("event_route[kazoo:consumer-event] is empty\n"); + } + kazoo_route_no=route_no; + } + +

Now removed route_lookup for kazoo:consumer-event as it may or may not exist.

@tao-communications commented on this pull request.

•  if (kazoo_route_no>=0) {
  

sprintf(buffer, "kazoo:consumer-event-%.*s-%.*s",ev_category.len, ev_category.s, ev_name.len, ev_name.s); - for (p=buffer ; *p; ++p) *p = tolower(*p); - for (p=buffer ; *p; ++p) if(*p == '_') *p = '-'; - if(kz_amqp_consumer_fire_event(buffer) != 0) { - sprintf(buffer, "kazoo:consumer-event-%.*s",ev_category.len, ev_category.s); for (p=buffer ; *p; ++p) *p = tolower(*p); for (p=buffer ; *p; ++p) if(*p == '_') *p = '-'; if(kz_amqp_consumer_fire_event(buffer) != 0) { - sprintf(buffer, "kazoo:consumer-event-%s-%s", key, subkey); + sprintf(buffer, "kazoo:consumer-event-%.*s",ev_category.len, ev_category.s); for (p=buffer ; *p; ++p) *p = tolower(*p); for (p=buffer ; *p; ++p) if(*p == '_') *p = '-'; if(kz_amqp_consumer_fire_event(buffer) != 0) { - sprintf(buffer, "kazoo:consumer-event-%s", key); + sprintf(buffer, "kazoo:consumer-event-%s-%s", key, subkey); for (p=buffer ; *p; ++p) *p = tolower(*p); for (p=buffer ; *p; ++p) if(*p == '_') *p = '-'; - if(kz_amqp_consumer_fire_event(buffer) != 0) { - sprintf(buffer, "kazoo:consumer-event"); - if(kz_amqp_consumer_fire_event(buffer) != 0) { - LM_ERR("kazoo:consumer-event not found\n"); - } - } + if(kz_amqp_consumer_fire_event(buffer) != 0) { + sprintf(buffer, "kazoo:consumer-event-%s", key); + for (p=buffer ; *p; ++p) *p = tolower(*p); + for (p=buffer ; *p; ++p) if(*p == '_') *p = '-'; + if(kz_amqp_consumer_fire_event(buffer) != 0) { + sprintf(buffer, "kazoo:consumer-event"); + if(kz_amqp_consumer_fire_event(buffer) != 0) { + LM_ERR("kazoo:consumer-event not found\n"); + } + } + } } } + } else { + keng = sr_kemi_eng_get(); + if(keng!=NULL) { + sip_msg_t *msg; + str evrtname = str_init("kazoo:consumer-event"); + + rtb = get_route_type(); + msg = faked_msg_next(); + if(sr_kemi_route(keng, msg, EVENT_ROUTE, &kazoo_event_callback, &evrtname)<0) { + LM_ERR("error running event route kemi callback\n"); + } + set_route_type(rtb); + } else { + LM_ERR("no event route or kemi callback found for execution\n"); + } } +

Now depending on using cfg or kemi: - kz_amqp_consumer_event() calls kz_amqp_consumer_event_cfg() or kz_amqp_consumer_event_kemi()

@tao-communications great job, thank you. i do have a final request, when that's done, please squash your commits into a single commit, `kazoo: add basic kemi support`

i feel that `kazoo_route_no` doesn't express well the intent for the variable which is to use `kemi` instead of `cfg`.

can you please rename `kazoo_route_no` to `kazoo_kemi_enabled` with default 0 and set it to 1 in [here](https://github.com/kamailio/kamailio/pull/2324/files#diff-52d37ad2407d869cdc...) and then use ``` if ( kazoo_kemi_enabled ) { do kemi routine } else { do cfg routine } ``` where applicable, for example in `fire_init_event` ``` if (kazoo_kemi_enabled) { return fire_init_event_kemi(); } else { return fire_init_event_cfg(); } ``

Thanks for reviewing! I made a new commit to address these. Please let me know if there are further things that need doing.

@lazedo: @tao-communications does not have commit access yet.

You can squash all commits via github merge button, just click on arrow down next to `Merge pull request` and choose `Squash and merge`. It is no need to have the squash done elsewhere.

I can do it if you prefer.

@miconda why did you dismissed my review ?

@lazedo - I dismissed the review because it was requesting an external squash, which is not necessary in my opinion. You can set the any commit message when you squash and merge via github.com web UI.

I have attempted a git squash and hopefully it's OK.

Merged #2324 into master.

Thank you very much @lazedo and @miconda for the review and support!