[Home]

Summary:ASTERISK-15947: [patch] cdr_mongodb
Reporter:Flavio Percoco Premoli (flaper87)Labels:
Date Opened:2010-04-12 14:54:25Date Closed:2010-07-06 15:11:00
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Addons/New Feature
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) cdr_mongodb.c
Description:Hey guys,

I created a MongoDB CDR backend for asterisk. It is hosted on github[0] and I tested it on my home asterisk installation.

It depends on mongo-c-driver[1] so I added some building/installation steps in the README file.

Please, let me know what you think and what can be improved.

I hope you like it.

[0] http://github.com/FlaPer87/cdr_mongodb
[1] http://github.com/mongodb/mongo-c-driver
Comments:By: Leif Madsen (lmadsen) 2010-04-13 12:40:12

Awesome! Thanks for the new feature! Once your license is approved we can start looking at the code. I'm importing it into our internal tracking system so we can try and get some development time to review your code.

Thanks again!
By: Flavio Percoco Premoli (flaper87) 2010-04-14 04:53:19

Np, I hope to do more ;)

Who's going to maintaine it (if approved)? can I maintain that code? how does the patch submission work in asterisk?
By: Flavio Percoco Premoli (flaper87) 2010-04-14 04:54:00

btw, thanks for considering the code and taking the time to review it.
By: Leif Madsen (lmadsen) 2010-04-15 12:24:14

If you're able to maintain the code, then we'd be thrilled to have you maintain it! (presuming it makes it as far as getting committed to Asterisk trunk). You'd likely be the expert on solving these issues anyways, so I think that could be arranged.

Right now we're in a holding pattern until a developer can review this code and move it forward. If in the meantime you do any updates to the code based on your own testing, then please feel free to upload as many corrections as necessary.

I'm going to set this to Ready for Testing right now. If you want, feel free to email the asterisk-users and/or asterisk-dev mailing lists to see if you can get any testers for this code.

Thanks!
By: Leif Madsen (lmadsen) 2010-04-15 12:25:41

In terms of submitting patches, you'd just open a new issue, and any new issues opened by someone else could be automatically assigned to you (so you'd know when someone opened a new issue for the MongoDB module).

We'd probably have to get you a certificate and all that jazz so you could commit your changes, but that won't be until after review and all that, so we've got some time yet to determine how that would happen.

Anyways, thanks again for the contribution!
By: Flavio Percoco Premoli (flaper87) 2010-04-16 02:02:13

Sure, I'm able to maintain the code and I would love too.

Please, Take your time to review the code and do everything that has to be done just let me know what happens =)

Thanks again for considering the code :)
By: Leif Madsen (lmadsen) 2010-04-20 11:21:04

Just an FYI for anyone coming across this:

http://try.mongodb.org/

Tutorial for MongoDB
By: Flavio Percoco Premoli (flaper87) 2010-04-27 11:11:57

Hi, =)

Is there any new about this?

Thanks
By: Leif Madsen (lmadsen) 2010-04-27 11:26:37

Hey flaper87!

This may take some time for any developers to get to it since it is a new feature, and we have over 600 open and active issues to work through in the Asterisk project alone.

Your patience is appreciated ;)
By: Flavio Percoco Premoli (flaper87) 2010-05-04 03:00:04

Awesome, I see that it now has a target version...

I'll probably doing some improvements, should I upload new versions here?

Thanks
By: Tilghman Lesher (tilghman) 2010-05-04 09:53:20

As long as this issue remains open, yes.
By: Tilghman Lesher (tilghman) 2010-05-04 10:15:36

A couple of things:

1. Your unload routine doesn't actually free the memory allocated by the module.  It should free all memory and unregister the CDR driver.
2. Please use ast_copy_string in preference to strncpy, as the former guarantees NULL termination.
3. You should be using the ast_str API to build strings, instead of strcpy/strcat.
4. I would prefer if your CDR driver scanned the structure of the database and created an insertion statement to match, instead of relying upon a static structure.  Most of the CDR drivers are done this way in trunk.
5. You need parentheses around ALL conditionals.  This would save you from a definite bug at lines 191-193 and 195-197.  The bson_append_string's are ALWAYS executed here, not as part of the conditional.
6. When using snprintf, the second parameter should always use the sizeof() construct.  Also, snprintf is safe to receive the entire size of the buffer.  No -1 offset is needed for this construct.
7. At line 297, you have a programming statement occur at the top of the block, which makes the declarations fall to within a block.  Declarations may only be at the very top of a block, per our programming guidelines.
By: Flavio Percoco Premoli (flaper87) 2010-05-05 04:46:59

Thanks a lot for letting me know those things, I'll fix them ASAP and upload the new version here...

Thanks again...
By: Tilghman Lesher (tilghman) 2010-05-26 17:02:06

As an FYI, time is running short for getting this new feature into 1.8.  Your updated patch needs to be ready by June 11th for a fair shake of being committed prior to the 1.8 feature freeze.
By: Paul Belanger (pabelanger) 2010-06-25 09:16:49

Ping!  Any updates? I think we'd like to get this into 1.8
By: Tilghman Lesher (tilghman) 2010-06-25 14:55:01

Unfortunately, due to a licensing concern, it appears that this driver is not a candidate for inclusion in Asterisk.  The FSF rendered a legal opinion that we support that the Apache license is not compatible with GPLv2, which is the license under which Asterisk is distributed.  If you want to create this as a project at forge.asterisk.org, distributed separately, that would certainly be an option, but we cannot consider it for inclusion with Asterisk due to this legal issue with the licenses.
By: Tilghman Lesher (tilghman) 2010-06-25 14:56:41

http://www.apache.org/licenses/GPL-compatibility.html