Summary:	ASTERISK-16652: [patch] merging categories from with static config to have realtime feature for register
Reporter:	Marcello Ceschia (marcelloceschia)	Labels:
Date Opened:	2010-09-06 08:19:27	Date Closed:	2011-12-01 12:44:42.000-0600
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Resources/res_config_pgsql
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) chan-sip_parsing-general_branch162.patch ( 1) res_config_pgsql-mergingcategories.patch ( 2) sip_register.sql
Description:	with this patch we are able to use satic and reatime configuration at the same time. Some example: in extconfig.conf we can add: sip_register.conf => pgsql,asterisk,sip_register in sip.conf just add under [general]: #include "sip_register.conf"
Comments:	By: Tilghman Lesher (tilghman) 2010-09-16 18:32:21 This patch would severely BREAK existing configurations, especially configurations where files have more than one context with the same name.  I simply cannot see why we would even want to contemplate making this change. By: Marcello Ceschia (marcelloceschia) 2010-09-17 05:37:05 Ok, I try to explain. If you would like to do everything in realtime and you dont want to touch static configurations, like sip.conf, you have no option for the registration strings e.g: [general] register => user:pass@provider.tld/xxx Normaly chan_sip does not parse the 2nd general context, so we are not able to use the "realtime register strings". Can you give me some example when this modification breaks existing configurations? Perhaps we will find an other solution. By: Tilghman Lesher (tilghman) 2010-09-17 10:21:33 The problem is that we sometimes have multiple contexts with the same name: In chan_iax2: [foo] type=user ... [foo] type=peer ... If it's the general context that you want merged, then I would suggest that you instead modify the sip channel driver to parse all such contexts in the configuration, not to merge contexts with the same name in the realtime backend driver.  This would be a fairly simple change and should not break any valid existing configurations.  The downside is that you'll need to modify each module for which you want that change. By: Marcello Ceschia (marcelloceschia) 2010-09-21 13:05:53 ok, you are right. I will move this to chan_sip By: Marcello Ceschia (marcelloceschia) 2010-09-28 08:20:42 I attached an alternative patch that only touches the chan_sip in parsing the general category. Please tell me if this is the right way By: Tilghman Lesher (tilghman) 2010-09-28 13:41:30 The first chunk of this patch appears to be an unrelated change.  Otherwise, looks okay. By: Marcello Ceschia (marcelloceschia) 2010-10-08 01:10:35 should we close this patch an open a new one related to chan_sip? By: Marcello Ceschia (marcelloceschia) 2010-10-13 08:55:25 @tilghman: how to proceed with that issue? By: Tilghman Lesher (tilghman) 2010-10-13 23:27:40 Your issue is in queue, please be patient, and we will get to it as time permits and developer resources become available. By: Leif Madsen (lmadsen) 2011-04-06 12:01:50 Marked as Ready for Testing. Did the original poster make the changes as requested by Tilghman? By: Leif Madsen (lmadsen) 2011-04-06 12:03:26 Additionally, I'd like to see this patch changed to not just be related to chan_sip stuff, but rather available for multiple other channel drivers. By: Marcello Ceschia (marcelloceschia) 2011-04-06 14:12:52 @ lmadsen I change my original patch after the hint of tilghman. By: Tilghman Lesher (tilghman) 2011-04-06 15:06:35 lmadsen:  there is no good way to do that without modifying those other drivers.  The issue is that we need it to be done in a way that is intelligent about the use of those contexts, which is not possible from the realtime level. By: Tilghman Lesher (tilghman) 2011-05-03 18:11:09 marcelloceschia:  You have a MIX of patches uploaded here.  Please upload a patch containing ONLY the changes agreed to above.  (i.e. the very first patch segment is an unrelated change.) By: Digium Subversion (svnbot) 2011-05-03 18:37:32 Repository: asterisk Revision: 316428 U   trunk/CHANGES U   trunk/channels/chan_sip.c ------------------------------------------------------------------------ r316428 | tilghman | 2011-05-03 18:36:35 -0500 (Tue, 03 May 2011) | 16 lines If multiple [general] contexts occur from sip.conf (usually due to external includes), merge them. The original implementation of this did the merging of all contexts with the same name in the realtime layer, but that implementation severely breaks drivers which use the same context name (e.g. iax.conf, type={peer,user}). Therefore, the implementation needs to do the merging for particular entries only, based upon what contexts would allow that in the channel driver itself. This implementation is for chan_sip only, but others could be added in the future. (closes issue ASTERISK-16652) Reported by: marcelloceschia Patches:       chan-sip_parsing-general_branch162.patch uploaded by marcelloceschia (license 1079) Tested by: tilghman ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=316428 By: Digium Subversion (svnbot) 2011-05-04 11:42:21 Repository: asterisk Revision: 316798 U   trunk/CHANGES U   trunk/channels/chan_sip.c ------------------------------------------------------------------------ r316798 | dvossel | 2011-05-04 11:42:20 -0500 (Wed, 04 May 2011) | 8 lines Reverts rev 316218 as it breaks parsing the [general] section of sip.conf. The functionality this patch attempts to achieve should already be possible using [general](+) in the config file. issue ASTERISK-16652 ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=316798 By: Marcello Ceschia (marcelloceschia) 2011-05-04 12:44:06 Ok, I dont know why "it breaks parsing the [general] section", we use this since one year. I also try the mentioned alternative, but this can not work, should I name the category general, or general+? Using the category='general' does not work as mentioned in the issue description. Using the category='general+' does: "Section 'general+' lacks type" It's right, that [general](+) do work, but in static configuration only! By: Matt Jordan (mjordan) 2011-12-01 12:44:42.286-0600 At this time, we do not feel that the patches in their current state are something that should be applied as a new feature to Asterisk.  If you'd like to rework the patch and resubmit it, please open a new issue and post your patch to reviewboard.