[asterisk-dev] [Code Review] 2846: Finish conversion script from sip.conf to pjsip.conf

[Mark Michelson]

> On Sept. 30, 2013, 4:19 p.m., Matt Jordan wrote:
> > /trunk/contrib/scripts/sip_to_pjsip/astconfigparser.py, lines 87-110
> > <https://reviewboard.asterisk.org/r/2846/diff/2/?file=46653#file46653line87>
> >
> >     I'm confused. Why do we reverse the sorted lists, then reverse them again when searching?
> 
> Mark Michelson wrote:
>     Kevin, can you comment on this one?
> 
> Kevin Harwell wrote:
>     So yeah maybe a better way to do this, but before adding of templates the given list is sorted, reversed and then added to, so that the "oldest" template definitions are guaranteed to be last.  So, when a get/search is issued it looks in the most recently added templates first.  During merges it is reversed so the "oldest" is merged first.
>     
>     Example:
>     [template_a](!)
>     allow=ulaw
>     allow=g711
>     context=default
>     
>     Defined later at some point:
>     
>     [template_a](!)
>     allow=alaw
>     context=test
>     
>     a merge of those two should output: allow=ulaw,g711,alaw and not allow=alaw,ulaw,g711 while a search should "get" a context of "test".
>     
>     Also see next issue's comment about why add_template should probably mimic add_defaults.

Okay, this makes sense to me. List is reverse-sorted so that searches are correct but then re-reversed for merges. I'll drop this finding.


> On Sept. 30, 2013, 4:19 p.m., Matt Jordan wrote:
> > /trunk/contrib/scripts/sip_to_pjsip/astconfigparser.py, lines 87-90
> > <https://reviewboard.asterisk.org/r/2846/diff/2/?file=46653#file46653line87>
> >
> >     This will add the elements in defaults to _defaults in reverse sorted order. So why not something like:
> >     
> >     self._defaults.extend(defaults.sort(reverse=True)
> >     
> >     
> >     Or exactly as add_templates was implemented.
> >
> 
> Kevin Harwell wrote:
>     Actually I think it would be better to make add_templates do what add_defaults does instead since they will result in two different lists, but I believe the add_defaults is what was the original intention (although I thought there was a use case that I can't think of now for doing add_templates differently).
>     
>     So for instance:
>     
>     add_default([1,2,3])
>     add_default([4,5,6])
>     
>     Should yield => [6,5,4,3,2,1]
>     
>     Where as:
>     
>     add_template([1,2,3])
>     add_template([4,5,6])
>     
>     Will yield => [3,2,1,6,5,4]

I'm going to make the modification to have add_templates match add_defaults' behavior.


- Mark


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2846/#review9859
-----------------------------------------------------------


On Sept. 26, 2013, 9 p.m., Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2846/
> -----------------------------------------------------------
> 
> (Updated Sept. 26, 2013, 9 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22374
>     https://issues.asterisk.org/jira/browse/ASTERISK-22374
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This finishes off the initial work on the conversion script from sip.conf to pjsip.conf. This addresses missing endpoint options, registration options, auth options, and transport options.
> 
> I ran the flake8 checker on the files, but I only corrected its complaints in the new sections I added in order to keep the diff reduced to the relevant changes. Unfortunately, this ended up being moot since I renamed the files and the directory that they are in.
> 
> 
> Diffs
> -----
> 
>   /trunk/contrib/scripts/sip_to_pjsip/astdicts.py PRE-CREATION 
>   /trunk/contrib/scripts/sip_to_pjsip/astconfigparser.py PRE-CREATION 
>   /trunk/contrib/scripts/sip_to_pjsip/sip_to_pjsip.py PRE-CREATION 
>   /trunk/contrib/scripts/sip_to_res_sip/astconfigparser.py 399909 
>   /trunk/contrib/scripts/sip_to_res_sip/astdicts.py 399909 
>   /trunk/contrib/scripts/sip_to_res_sip/sip_to_res_sip.py 399909 
> 
> Diff: https://reviewboard.asterisk.org/r/2846/diff/
> 
> 
> Testing
> -------
> 
> Tested against various sip.conf files to ensure that generated sections had the expected data in them.
> 
> 
> Thanks,
> 
> Mark Michelson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20131015/fbc06a22/attachment-0001.html>