Summary:	ASTERISK-03306: [patch] Configurable options for cdr_csv
Reporter:	Michael Giagnocavo (aginamu)	Labels:
Date Opened:	2005-01-18 21:44:41.000-0600	Date Closed:	2011-06-07 14:05:13
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	CDR/cdr_csv
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) cdr_csv_fixed_noflags.diff.txt ( 1) cdr_csv_flagmacros.diff.txt ( 2) cdr_csv_settings.diff.txt ( 3) cdr_csv.conf ( 4) cdr_csv.fixed.conf
Description:	I've made the cdr_csv options read from a config, rather than #defines (but maintained backwards compat). I've also made more things configurable, such as the filename and directory. Finally, I added settings to write to a log file based on the time (so you can have hourly, daily, or monthly logs), and allow the times used to be localtime, or UTC. ****** ADDITIONAL INFORMATION ****** Disclaimer on file, AFAIK (faxed a few days ago).
Comments:	By: Mark Spencer (markster) 2005-01-18 23:38:01.000-0600 Although the use of single bit variables is certainly valid, we have established a set of flag macros (ast_set_flag, ast_clear_flag, ast_set2_flag, ast_copy_flags, etc) which are preferred for consistancy.  They do have a slight technical advantage which is the ability to copy several in a single routine (ast_copy_flags)... By: Michael Giagnocavo (aginamu) 2005-01-19 04:12:10.000-0600 Yes. I should note that I did not use those on purpose, as I wanted this to work on the stable branch as well. Also, since it was global settings, versus copy-needing flags, I didn't think it would hurt. In fact, I was thinking about moving all the settings into that struct (logdir, masterfile) just to make it nice and clean. By: Andrey S Pankov (casper) 2005-01-19 07:00:20.000-0600 If you want your code be in cvs head you should use ast_xxx_flag functions as markster says. No chance this will be in 1.0 [as per drumkilla]... :( 2c about the code: a) no need to initialize settings twice in configure(), may be done in more elegant way; b) configure() need ast_destroy(config) before return; c) no need to re-initialize date format each time in build_log_time(), why not to move format in e.g. settings structure? d) if you are sure operations are atomic now then there is no need for e.g. "if (mf) fclose(mf);" checking and mf declared outside writefile(); By: Michael Giagnocavo (aginamu) 2005-01-19 08:54:33.000-0600 Sorry, I didn't mean to imply that it'd go into the REAL 1.0. I just wanted to make sure one could use it on 1.0 production servers. I'll make the fixes you mention, casper, and rewrite it with the flag macros too. As far as "if (mf) fclose(mf)", I just moved that from the original code. Thanks! By: Michael Giagnocavo (aginamu) 2005-01-19 12:33:45.000-0600 OK, I uploaded the struct-based one with some fixed and the flagified one with fixes. Compare 'em and see the readability impact. Also fixed the default to create multiple files (one per account code), since that's the current. Casper mentioned some locking issues. I'm using a lock because AFAIK, a reload could happen in the middle of a log, and that could change the filename. By: Mark Spencer (markster) 2005-02-06 21:31:45.000-0600 Do you think perhaps that every field should be a bit in a bitmask that could optionally be included? (I don't think the order is important, just having the bitmask would likely be sufficient). By: Michael Giagnocavo (aginamu) 2005-02-06 21:35:55.000-0600 I suppose that'd be ok. I think it'd make more sense to have each field represented as a one-bit field in a structure, as this isn't something that needs to be represented as a bitmask or persisted. By: Clod Patry (junky) 2005-04-09 11:30:58 Where are we with this bug? /Housekeeping. By: Mark Spencer (markster) 2005-04-10 22:29:57 To some degree this bug is obsoleted by the cdr_custom.... By: Michael Giagnocavo (aginamu) 2005-04-11 20:10:17 Yea, I agree that things should progress towards cdr_custom instead. By: Clod Patry (junky) 2005-04-20 11:52:48 This could be close. See ASTERISK-3515 too. By: Kevin P. Fleming (kpfleming) 2005-04-20 11:58:51 This was not 'fixed'...