Re: Submitting a patch to Kannel: best practices?

Well, it's a tradeoff, as usual.

If you get a complex cluster of services, usually you either manage them altogether (and lose granularity), or manage them individually (and each time you need to change something on all of them you need to do it X times).

Now, a good compromise would be to be able to optionally target a specific instance by passing the smsbox-id. If you don't pass it, the command is delivered to all the connected smsboxes, but if you pass it the command is only passed to that specific smsbox instance.

Regards,
--
Alejandro Guerrieri
[email protected]

On 17/12/2010, at 19:44, Nikos Balkanas wrote:

> Maybe, but it doesn't scale well. Consider that you may have 2-3 smsboxes, an sqlbox and an smppbox (all connected as smsboxes to bb). Next you might want to be able to change dynamically log levels to each one, or dynamically manage sendsms accounts. You will end up with a bb admin page a mile long. Besides, sqlbox has different admin requirements than smppbox or smsbox and bb cannot tell them apart.
> 
> BR,
> Nikos
> ----- Original Message ----- From: Alexander Malysh
> To: Nikos Balkanas
> Cc: David McCann ; [email protected]
> Sent: Friday, December 17, 2010 5:04 PM
> Subject: Re: Submitting a patch to Kannel: best practices?
> 
> 
> Hi,
> 
> 
> it would be better to let bearerbox tell smsbox to reload lists. admin message already exists for communication
> between bearerbox and smsbox (see restart command as example).
> 
> 
> Then you don't need extra admin interface for smsbox.
> 
> 
> Thanks,
> Alexander Malysh
> 
> 
> Am 17.12.2010 um 14:49 schrieb Nikos Balkanas:
> 
> 
> Hi David,
> 
> 
> 
> Yes. bb has its own black/white lists for incoming MO traffic, similar to smsbox's MT. These can be reloaded on the fly through its http admin interface. Another very useful feature is that it allows you to change log-levels on the fly. Very useful for debugging.
> 
> 
> Check gw/bb_http.c: httpd_commands for sources.
> 
> 
> I am not talking about bearerbox telling smsbox to reload its lists. I am talking for a separate http admin for smsbox, like the one in bb. Otherwise you would have major restructuring to get it through the admin *Msg, and it is not worth it.
> 
> 
> BR,
> Nikos
> 
> 
> On Fri, Dec 17, 2010 at 1:07 PM, David McCann <[email protected]> wrote:
> 
> Hi Nikos--
> 
> 
> Thank you for the promt reply!  I completely agree that this makes more sense as a bearerbox, admin command.  I initially added it as such, but then realized that in fact, all the translations logic sat within the smsbox process, rather than the bearerbox.  Adding it directly as an smsbox command removes any need for any communication between the bearerbox and the smsbox.
> 
> 
> But I agree it still feels like a hack, in terms of where a user would expect to send a command such as "refresh list."  If you could point me to a pattern in the code where the bearerbox communicates with the smsbox in a similar fashion, it'd be a huge help and I'd be happy to re-submit my patch with it working in that manner.  The current diff is still attached to the feature request, but I've attached it here as well (currently in two patches, src and doc) just for review, as we discuss this alternate approach.
> 
> 
> In regards to your comment about bearerbox handling this on the fly through its admin HTTP interface...I'm not quite sure I follow?  I know this service-level refreshing functionality doesn't currently exist, are you just referring to similar functionality that exists in bearerbox?  Forgive my confusion.
> 
> 
> Thanks again,
> --dm
> 
> 
> 
> 
> On Fri, Dec 17, 2010 at 1:07 PM, Nikos Balkanas <[email protected]> wrote:
> 
> Hi,
> 
> 
> 
> Usually people just post the patch to the devel list with subject: Patch: <filename>. Patch is attached as a diff of the file(s) from latest svn sources, followed by a brief description in the body.
> 
> 
> Wrt to your proposed solution, I am not very much in favor. In bearerbox this is handle on the fly through its admin HTTP interface. I think a similar approach would be best for smsbox.
> 
> 
> BR,
> Nikos
> 
> 
> On Fri, Dec 17, 2010 at 8:10 AM, David McCann <[email protected]> wrote:
> 
> Greetings all!
> 
> 
> I've been a kannel user for years, but within the past week I've just started to work directly with the code.  I had a particular need which I couldn't find a good workaround for:
> 
> 
> My current deployment with kannel uses whitelisting to dispatch messages to various running web applications (at the sms-service group level), however users do update their contact information from time to time, meaning the whitelist needs to updated, preferrably without the smsbox having to be restarted entirely.
> 
> 
> I've added a new command to the list of available commands for SMSBox, namely /cgi-bin/refreshlist, and (maybe a little overzealously) created an issue: https://redmine.kannel.org/issues/584
> 
> 
> And submitted my patch there.
> 
> 
> Given that I'm pretty new to this community, I'm wondering if anyone can advise me on the best/most convenient way to submit a patch for incorporation into the code?
> 
> 
> Thanks in advance,
> David McCann
> T4D, UNICEF Uganda 
>