Summary:	ASTERISK-23823: [patch] Option to keep queuerules in realtime
Reporter:	Michael K. (michaelk)	Labels:
Date Opened:	2014-06-05 06:34:32	Date Closed:	2014-08-10 19:08:56
Priority:	Minor	Regression?
Status:	Closed/Complete	Components:	Applications/app_queue
Versions:	SVN 1.8.27.0	Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) app_queue.c_realtime_trunk.patch ( 1) app_queue.patch ( 2) queue_rules.sql
Description:	This patch the ability (optional) to keep queuerules in realtime (rules would be loaded from file too) To test add to queuerules.conf : [general] realtime_rules = yes and add the realtime setting to extconfig.conf, for example in my case it's mysql and it looks like(will attach the .sql with create for table): queue_rules => mysql,asterisk,queue_rules in table as you can see you need to specify rule name and as in regular rules you can use relative setings for min and max (with "-" and "+", e.g. "+100") If you use realtime, the rules will be always reloaded (no cache option here), in other words it would delete all rules and re-add them from realtime and file. It's important to understand that with realtime on it would reload rules from file doesn't matter if file was or was not change. While with the option off it would act exactly like it was before patch (file would not be reloaded if it was not changed)
Comments:	By: Michael K. (michaelk) 2014-06-05 06:36:04.265-0500 This is the SQL create statement for table for rules for MySQL: By: Michael K. (michaelk) 2014-06-05 06:39:38.885-0500 The patch itself By: Matt Jordan (mjordan) 2014-06-05 09:27:12.146-0500 A few comments: # New features should be made against Asterisk trunk. As this issue was made against Asterisk 1.8, I'm assuming the patch was made against that version of Asterisk. Please provide a patch that merges cleanly with Asterisk trunk. # Taking a quick cursory glance at the patch, there appear to be numerous [coding guideline violations|https://wiki.asterisk.org/wiki/display/AST/Coding+Guidelines]. Please update the patch per the guidelines on the wiki before submitting it for inclusion. # Subtle changes in {{app_queue}} tend to result in regressions. This is particularly true when realtime is involved. What testing was done for this new feature? Can tests be written for the Asterisk Test Suite that verify this feature? By: Michael K. (michaelk) 2014-06-09 10:44:29.442-0500 1) I'm preparing for trunk as requested. And yes the patch was made against 1.8.27 2) As if it's tabs/extra spaces and other minor problems, i fixed those on the patch for trunk. If there are more serious problems, i would like to get help, feedback and guidance as i'm junior dev and may missed something or made the wrong choices. 3) Regression only if the reload would always reload file and realtime, i'm not sure if realtime takes more time than read from file. So if you say so, it maybe true. But this is optional and i do not think it makes much change, but it's easier (at least for us) to keep settings in (for example, again in our case) in db(mysql) The testing was done on development server with different cases (tried not proper values in fields and so on), after more tests we plan to use it in production, so we can get better feedback (though not sure if it would be only this patch or also patch for changing other parameters due time(announcement for example) which i plan to submit too later). I'm not sure if Test Suite is relevant here, if i understand it right, as all the changes are made is the loading of the rules ( the source) and only relevant on reload/load of module. Or i got the wrong idea about Test Suite. By: Michael K. (michaelk) 2014-06-09 11:07:51.572-0500 this is the patch against trunk (revision 415442) generated by "svn diff" By: Matt Jordan (mjordan) 2014-06-10 11:48:50.902-0500 Please go ahead and post the patch to review board for peer review. Guidelines for doing so and instructions on using review board can be found on the Asterisk wiki: https://wiki.asterisk.org/wiki/display/AST/Code+Review By: Michael K. (michaelk) 2014-06-11 05:05:10.113-0500 https://reviewboard.asterisk.org/r/3607/ By: Michael K. (michaelk) 2014-07-27 09:55:29.850-0500 Still waiting for more reviews and approval on reviewboard:(