[prev in list] [next in list] [prev in thread] [next in thread]
List: asterisk-dev
Subject: Re: [asterisk-dev] Fwd: Code Review request of func_redis module
From: Sergio Medina Toledo <gestoip () ull ! edu ! es>
Date: 2015-07-10 14:42:14
Message-ID: CANMERYLwZYugUV_8Enu46Chs15g09WcSNEXnu+zQr2TqcS+3Zg () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
Hi,
I have finished the REDIS_PUBLISH function, it is in the repo (
https://github.com/tic-ull/func_redis), it took me more time find the time
to do it than the time to do it,
all the fedback is welcome.
2015-07-08 10:49 GMT+01:00 Sergio Medina Toledo <gestoip@ull.edu.es>:
> Hi,
>
> There isn't any drawback with that, all the pull requests and
> contributions are welcome.
>
> I would add a function called REDIS_PUBLISH, that may work like that
>
> Set(REDIS_PUBLISH(redis_channel)=message)
>
> If I have some time this afternoon i will try to implement it.
>
> If you want something like the "redis_cmd" in kamailio i'm not gonna do
> that, not at the moment, maybe in the future, but if you want to implement
> it, it will be welcome.
>
> Regards.
>
> 2015-07-08 9:20 GMT+01:00 Ludovic Gasc <gmludo@gmail.com>:
>
>> Hi Sergio,
>>
>> I've added in my professional schedule to test your module this summer.
>> I'll open issue or pull request for specific remarks.
>> However, one of first thing I'll need quickly, is to push any command to
>> redis, like with kamailio module:
>> http://kamailio.org/docs/modules/4.4.x/modules/ndb_redis.html#idp206176
>>
>> Do you see any drawbacks if I add a new function to do that in func_redis
>> ?
>>
>> My idea is to use PUBLISH command to push only informations I need to my
>> daemons, instead of to parse lot of messages via AMI, to be more efficient
>> at high level of calls: http://redis.io/commands/publish
>>
>> Regards.
>>
>> --
>> Ludovic Gasc (GMLudo)
>> http://www.gmludo.eu/
>>
>> 2015-05-13 12:38 GMT+02:00 Sergio Medina Toledo <gestoip@ull.edu.es>:
>>
>>> Great, then there isn't any problem, for me is fantastic having a tester
>>> that can report bugs.
>>>
>>> 2015-05-12 20:38 GMT+01:00 Ludovic Gasc <gmludo@gmail.com>:
>>>
>>>> Don't worry, I understand it isn't production ready, I'll test that
>>>> only on a dev instance.
>>>> I don't know yet if I'll have skills to help you with pull requests,
>>>> but at least, with tests and eventual bug reports.
>>>>
>>>> For now, it isn't really critical, because I've a daemon that plays as
>>>> a Redis proxy for Asterisk, but I vote always for simplifications, if only
>>>> I don't have efficiency impacts.
>>>>
>>>> --
>>>> Ludovic Gasc (GMLudo)
>>>> http://www.gmludo.eu/
>>>>
>>>> 2015-05-12 9:04 GMT+02:00 Sergio Medina Toledo <gestoip@ull.edu.es>:
>>>>
>>>>> The part of the internal API that I used haven't changed from 11 to 13
>>>>> so it should work without any problem. But I haven't tested the module
>>>>> intensively and I'm not sure if it is thread safe, so I don't recommend use
>>>>> it in production servers right now. Today I'm gonna install the module in
>>>>> an asterisk 11 active-passive cluster of preproduction to test it better,
>>>>> after that I will expose my conclusions here, if some one that have more
>>>>> experience developing in asterisk can look at the code and confirm that it
>>>>> is thread safe i will be very grateful.
>>>>>
>>>>> 2015-05-11 21:22 GMT+01:00 Ludovic Gasc <gmludo@gmail.com>:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Your library is very interesting, because we already use Redis as a
>>>>>> cache for some stuffs.
>>>>>> For now, I've made a FastAGI server to serve Redis data, but it
>>>>>> should be faster to read Redis data directly.
>>>>>>
>>>>>> Do you think I could test your module on an Asterisk 13, or the
>>>>>> Asterisk internal API is too different to be used with another version than
>>>>>> 11 ?
>>>>>>
>>>>>> Regards.
>>>>>>
>>>>>> Ludovic Gasc (GMLudo)
>>>>>> http://www.gmludo.eu/
>>>>>> On 11 May 2015 09:14, "GESTION DE TELEFONIA IP" <gestoip@ull.edu.es>
>>>>>> wrote:
>>>>>>
>>>>>>> Hi all, I have developed an asterisk module to interact with redis
>>>>>>> from the dialplan, the project is motivated by the need to share
>>>>>>> information between different asterisk in both active-active and
>>>>>>> active-passive scheme, in an easy way like AstDB. You can use an
>>>>>>> agi script or a system call to a script to use redis from the dialplan but
>>>>>>> the performance is low compared to a asterisk module and the integration is
>>>>>>> worst. I think that maybe this module can be interesting to the
>>>>>>> community,
>>>>>>>
>>>>>>> It's the first time that I develop a asterisk module so If some one
>>>>>>> can make a code review to the module to see if there are use of insecure
>>>>>>> functions or something that can crash asterisk, I will be very grateful,
>>>>>>> the module is under GPLv2 License in this repo
>>>>>>> https://github.com/tic-ull/func_redis. At the moment the module is
>>>>>>> shipped outside the asterisk project, as a Cmake project. You can
>>>>>>> see the instructions to install and to use it in the README.md of the repo.
>>>>>>>
>>>>>>> The english isn't my native language so sorry if there ara some
>>>>>>> writing mistakes.
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> _____________________________________________________________________
>>>>>>> -- Bandwidth and Colocation Provided by http://www.api-digital.com
>>>>>>> --
>>>>>>>
>>>>>>> asterisk-dev mailing list
>>>>>>> To UNSUBSCRIBE or update options visit:
>>>>>>> http://lists.digium.com/mailman/listinfo/asterisk-dev
>>>>>>>
>>>>>>
>>>>>> --
>>>>>> _____________________________________________________________________
>>>>>> -- Bandwidth and Colocation Provided by http://www.api-digital.com --
>>>>>>
>>>>>> asterisk-dev mailing list
>>>>>> To UNSUBSCRIBE or update options visit:
>>>>>> http://lists.digium.com/mailman/listinfo/asterisk-dev
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> _____________________________________________________________________
>>>>> -- Bandwidth and Colocation Provided by http://www.api-digital.com --
>>>>>
>>>>> asterisk-dev mailing list
>>>>> To UNSUBSCRIBE or update options visit:
>>>>> http://lists.digium.com/mailman/listinfo/asterisk-dev
>>>>>
>>>>
>>>>
>>>> --
>>>> _____________________________________________________________________
>>>> -- Bandwidth and Colocation Provided by http://www.api-digital.com --
>>>>
>>>> asterisk-dev mailing list
>>>> To UNSUBSCRIBE or update options visit:
>>>> http://lists.digium.com/mailman/listinfo/asterisk-dev
>>>>
>>>
>>>
>>> --
>>> _____________________________________________________________________
>>> -- Bandwidth and Colocation Provided by http://www.api-digital.com --
>>>
>>> asterisk-dev mailing list
>>> To UNSUBSCRIBE or update options visit:
>>> http://lists.digium.com/mailman/listinfo/asterisk-dev
>>>
>>
>>
>> --
>> _____________________________________________________________________
>> -- Bandwidth and Colocation Provided by http://www.api-digital.com --
>>
>> asterisk-dev mailing list
>> To UNSUBSCRIBE or update options visit:
>> http://lists.digium.com/mailman/listinfo/asterisk-dev
>>
>
>
[Attachment #5 (text/html)]
<div dir="ltr">Hi,<br><br>I have finished the REDIS_PUBLISH function, it is in the \
repo (<a href="https://github.com/tic-ull/func_redis">https://github.com/tic-ull/func_redis</a>), \
it took me more time find the time to do it than the time to do it,<br>all the \
fedback is welcome.<br></div><div class="gmail_extra"><br><div \
class="gmail_quote">2015-07-08 10:49 GMT+01:00 Sergio Medina Toledo <span \
dir="ltr"><<a href="mailto:gestoip@ull.edu.es" \
target="_blank">gestoip@ull.edu.es</a>></span>:<br><blockquote class="gmail_quote" \
style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div \
dir="ltr">Hi,<br><br>There isn't any drawback with that, all the pull requests \
and contributions are welcome.<br><br>I would add a function called REDIS_PUBLISH, \
that may work like that <br><br>Set(REDIS_PUBLISH(redis_channel)=message)<br><br>If I \
have some time this afternoon i will try to implement it.<br><br>If you want \
something like the "redis_cmd" in kamailio i'm not gonna do that, not \
at the moment, maybe in the future, but if you want to implement it, it will be \
welcome.<br><br>Regards.<br></div><div class="HOEnZb"><div class="h5"><div \
class="gmail_extra"><br><div class="gmail_quote">2015-07-08 9:20 GMT+01:00 Ludovic \
Gasc <span dir="ltr"><<a href="mailto:gmludo@gmail.com" \
target="_blank">gmludo@gmail.com</a>></span>:<br><blockquote class="gmail_quote" \
style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div \
dir="ltr">Hi Sergio,<div><br></div><div>I've added in my professional schedule to \
test your module this summer.</div><div>I'll open issue or pull request for \
specific remarks.</div><div>However, one of first thing I'll need quickly, is to \
push any command to redis, like with kamailio module:</div><div><a \
href="http://kamailio.org/docs/modules/4.4.x/modules/ndb_redis.html#idp206176" \
target="_blank">http://kamailio.org/docs/modules/4.4.x/modules/ndb_redis.html#idp206176</a><br></div><div><br></div><div>Do \
you see any drawbacks if I add a new function to do that in func_redis \
?</div><div><br></div><div>My idea is to use PUBLISH command to push only \
informations I need to my daemons, instead of to parse lot of messages via AMI, to be \
more efficient at high level of calls: <a href="http://redis.io/commands/publish" \
target="_blank">http://redis.io/commands/publish</a></div><div><br></div><div>Regards.</div></div><div \
class="gmail_extra"><span><br clear="all"><div><div><div dir="ltr"><div>--<br><div \
style="font-size:small"><div>Ludovic Gasc (GMLudo)</div></div><div \
style="font-size:small"><a href="http://www.gmludo.eu/" \
target="_blank">http://www.gmludo.eu/</a></div></div></div></div></div> \
<br></span><div><div><div class="gmail_quote">2015-05-13 12:38 GMT+02:00 Sergio \
Medina Toledo <span dir="ltr"><<a href="mailto:gestoip@ull.edu.es" \
target="_blank">gestoip@ull.edu.es</a>></span>:<br><blockquote class="gmail_quote" \
style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div \
dir="ltr">Great, then there isn't any problem, for me is fantastic having a \
tester that can report bugs.<br></div><div><div><div class="gmail_extra"><br><div \
class="gmail_quote">2015-05-12 20:38 GMT+01:00 Ludovic Gasc <span dir="ltr"><<a \
href="mailto:gmludo@gmail.com" \
target="_blank">gmludo@gmail.com</a>></span>:<br><blockquote class="gmail_quote" \
style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div \
dir="ltr">Don't worry, I understand it isn't production ready, I'll test \
that only on a dev instance.<div>I don't know yet if I'll have skills to help \
you with pull requests, but at least, with tests and eventual bug \
reports.</div><div><br></div><div>For now, it isn't really critical, because \
I've a daemon that plays as a Redis proxy for Asterisk, but I vote always for \
simplifications, if only I don't have efficiency impacts.</div></div><div \
class="gmail_extra"><br clear="all"><div><div><div dir="ltr"><div>--<span><br><div \
style="font-size:small"><div>Ludovic Gasc (GMLudo)</div></div><div \
style="font-size:small"><a href="http://www.gmludo.eu/" \
target="_blank">http://www.gmludo.eu/</a></div></span></div></div></div></div><div><div>
<br><div class="gmail_quote">2015-05-12 9:04 GMT+02:00 Sergio Medina Toledo <span \
dir="ltr"><<a href="mailto:gestoip@ull.edu.es" \
target="_blank">gestoip@ull.edu.es</a>></span>:<br><blockquote class="gmail_quote" \
style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div \
dir="ltr">The part of the internal API that I used haven't changed from 11 to 13 \
so it should work without any problem. But I haven't tested the module \
intensively and I'm not sure if it is thread safe, so I don't recommend use \
it in production servers right now. Today I'm gonna install the module in an \
asterisk 11 active-passive cluster of preproduction to test it better, after that I \
will expose my conclusions here, if some one that have more experience developing in \
asterisk can look at the code and confirm that it is thread safe i will be very \
grateful.<br></div><div><div><div class="gmail_extra"><br><div \
class="gmail_quote">2015-05-11 21:22 GMT+01:00 Ludovic Gasc <span dir="ltr"><<a \
href="mailto:gmludo@gmail.com" \
target="_blank">gmludo@gmail.com</a>></span>:<br><blockquote class="gmail_quote" \
style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div \
dir="ltr"><p dir="ltr">Hi,</p> <p dir="ltr">Your library is very interesting, because \
we already use Redis as a cache for some stuffs.<br>For now, I've made a FastAGI \
server to serve Redis data, but it should be faster to read Redis data \
directly.</p><p>Do you think I could test your module on an Asterisk 13, or the \
Asterisk internal API is too different to be used with another version than 11 \
?</p><p>Regards.</p> <p dir="ltr">Ludovic Gasc (GMLudo)<br>
<a href="http://www.gmludo.eu/" target="_blank">http://www.gmludo.eu/</a></p>
<div class="gmail_quote"><div><div>On 11 May 2015 09:14, "GESTION DE TELEFONIA \
IP" <<a href="mailto:gestoip@ull.edu.es" \
target="_blank">gestoip@ull.edu.es</a>> wrote:<br \
type="attribution"></div></div><blockquote class="gmail_quote" style="margin:0 0 0 \
.8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div><div dir="ltr"><span>Hi \
all, I have developed an asterisk module to interact with redis from the dialplan, \
</span><span>the project is motivated by the need to share information between \
different asterisk in both active-active and active-passive scheme, in an easy way \
like AstDB. </span><span>You can use an agi script or a system call to a script to \
use redis from the dialplan but the performance is low compared to a asterisk module \
and the integration is worst. I </span>think that maybe this module can be \
interesting to the community, <br><div class="gmail_quote"><div \
dir="ltr"><div><span><span></span><br>It's the first time that I develop a \
asterisk module so If some one can make a code review to the module to see if there \
are use of insecure functions or something that can crash asterisk, I will
be very grateful, the module is under GPLv2 License in this repo <a \
href="https://github.com/tic-ull/func_redis" \
target="_blank">https://github.com/tic-ull/func_redis</a>. At the moment the module \
is shipped outside the asterisk project, as a Cmake project.</span><span><span> You \
can see the instructions to install and to use it in the README.md of the \
repo.</span><br><br></span><span>The english isn't my native language so sorry if \
there ara some writing mistakes.<br></span></div></div> </div><br></div>
<br></div></div><span><font color="#888888">--<br>
_____________________________________________________________________<br>
-- Bandwidth and Colocation Provided by <a href="http://www.api-digital.com" \
target="_blank">http://www.api-digital.com</a> --<br> <br>
asterisk-dev mailing list<br>
To UNSUBSCRIBE or update options visit:<br>
<a href="http://lists.digium.com/mailman/listinfo/asterisk-dev" \
target="_blank">http://lists.digium.com/mailman/listinfo/asterisk-dev</a><br></font></span></blockquote></div>
</div>
<br>--<br>
_____________________________________________________________________<br>
-- Bandwidth and Colocation Provided by <a href="http://www.api-digital.com" \
target="_blank">http://www.api-digital.com</a> --<br> <br>
asterisk-dev mailing list<br>
To UNSUBSCRIBE or update options visit:<br>
<a href="http://lists.digium.com/mailman/listinfo/asterisk-dev" \
target="_blank">http://lists.digium.com/mailman/listinfo/asterisk-dev</a><br></blockquote></div><br></div>
</div></div><br>--<br>
_____________________________________________________________________<br>
-- Bandwidth and Colocation Provided by <a href="http://www.api-digital.com" \
target="_blank">http://www.api-digital.com</a> --<br> <br>
asterisk-dev mailing list<br>
To UNSUBSCRIBE or update options visit:<br>
<a href="http://lists.digium.com/mailman/listinfo/asterisk-dev" \
target="_blank">http://lists.digium.com/mailman/listinfo/asterisk-dev</a><br></blockquote></div><br></div></div></div>
<br>--<br>
_____________________________________________________________________<br>
-- Bandwidth and Colocation Provided by <a href="http://www.api-digital.com" \
target="_blank">http://www.api-digital.com</a> --<br> <br>
asterisk-dev mailing list<br>
To UNSUBSCRIBE or update options visit:<br>
<a href="http://lists.digium.com/mailman/listinfo/asterisk-dev" \
target="_blank">http://lists.digium.com/mailman/listinfo/asterisk-dev</a><br></blockquote></div><br></div>
</div></div><br>--<br>
_____________________________________________________________________<br>
-- Bandwidth and Colocation Provided by <a href="http://www.api-digital.com" \
rel="noreferrer" target="_blank">http://www.api-digital.com</a> --<br> <br>
asterisk-dev mailing list<br>
To UNSUBSCRIBE or update options visit:<br>
<a href="http://lists.digium.com/mailman/listinfo/asterisk-dev" rel="noreferrer" \
target="_blank">http://lists.digium.com/mailman/listinfo/asterisk-dev</a><br></blockquote></div><br></div></div></div>
<br>--<br>
_____________________________________________________________________<br>
-- Bandwidth and Colocation Provided by <a href="http://www.api-digital.com" \
rel="noreferrer" target="_blank">http://www.api-digital.com</a> --<br> <br>
asterisk-dev mailing list<br>
To UNSUBSCRIBE or update options visit:<br>
<a href="http://lists.digium.com/mailman/listinfo/asterisk-dev" rel="noreferrer" \
target="_blank">http://lists.digium.com/mailman/listinfo/asterisk-dev</a><br></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
--
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic