Re: Submitting a patch to Kannel: best practices?

"Nikos Balkanas" <[email protected]>
Newsgroups gmane.comp.mobile.kannel.devel
Message-ID <472CE381075A4B2C93E99459B1C2972C@drwho>
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.