Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

> On March 18, 2014, 12:43 p.m., wdoekes wrote: > > /branches/12/res/res_pjsip/config_system.c, lines 200-206 > > > > > > No ao2_ref(discovered_nameservers, -1); here? Fixed in SVN, cheers! - Joshua ---

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3343/#review11269 --- I realise this was committed already, but: /branches/12/res/r

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3343/ --- (Updated March 17, 2014, 5:53 p.m.) Status -- This change has been ma

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3343/#review11257 --- Ship it! Ship It! - Sean Bright On March 17, 2014, 1:15 p.m

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3343/#review11253 --- Ship it! There has been a long and healthy debate on this patc

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3343/ --- (Updated March 17, 2014, 1:15 p.m.) Review request for Asterisk Developers

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

On 03/13/2014 10:13 PM, Sean Bright wrote: > In order to make this "channel agnostic" you have three (equally bad) options: > > 1. Replace Asterisk's internal DNS facilities with PJLIB's, creating a > mandatory > dependency on PJSIP. > 2. Roll a shiny new DNS API into Asterisk that supports

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

On 14 Mar 2014, at 21:29, Jared Smith wrote: > > On Thu, Mar 13, 2014 at 3:34 PM, Olle E Johansson > wrote: > For every poorly designed bad feature I can think of I can find a large > number of Asterisk users that want it. It's simply not a good argument. We > create this software and need

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

On Thu, Mar 13, 2014 at 3:34 PM, Olle E Johansson wrote: > For every poorly designed bad feature I can think of I can find a large > number of Asterisk users that want it. It's simply not a good argument. We > create this software and need some sort of architecture when we do. > > I think you're

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

On Fri, Mar 14, 2014 at 1:19 PM, Matthew Jordan wrote: > On Fri, Mar 14, 2014 at 9:02 AM, Olle E. Johansson wrote: >> >> It would mean continuing to maintain Asterisk's pjproject fork until those >> changes were (hopefully) accepted upstream, released, and then waiting for >> the rpm/deb packages

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

On Fri, Mar 14, 2014 at 12:49 PM, Joshua Colp wrote: > Matthew Jordan wrote: > > > > >>> My problem is when I get arguments like "it's there in PJIP so we >>> have to use it" or "we can't do anything because of PJSIP". >> >> >> That's not my argument at all. >> >> My argument is thus: >> >> * PJS

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

Matthew Jordan wrote: My problem is when I get arguments like "it's there in PJIP so we have to use it" or "we can't do anything because of PJSIP". That's not my argument at all. My argument is thus: * PJSIP provides DNS resolution that far exceeds what is capable in Asterisk today and doe

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

On Fri, Mar 14, 2014 at 9:29 AM, Olle E. Johansson wrote: > > On 14 Mar 2014, at 15:22, Sean Bright wrote: > >> On 3/14/2014 10:02 AM, Olle E. Johansson wrote: It would mean continuing to maintain Asterisk's pjproject fork until those changes were (hopefully) accepted upstream, release

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

On Fri, Mar 14, 2014 at 9:02 AM, Olle E. Johansson wrote: > > It would mean continuing to maintain Asterisk's pjproject fork until those > changes were (hopefully) accepted upstream, released, and then waiting for > the rpm/deb packages to catch up. Not to mention that someone would > actually ha

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

On 14 Mar 2014, at 16:57, Paul Belanger wrote: > On Fri, Mar 14, 2014 at 10:29 AM, Olle E. Johansson wrote: >> >> On 14 Mar 2014, at 15:22, Sean Bright wrote: >> >>> On 3/14/2014 10:02 AM, Olle E. Johansson wrote: > It would mean continuing to maintain Asterisk's pjproject fork until >>

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

On Fri, Mar 14, 2014 at 10:29 AM, Olle E. Johansson wrote: > > On 14 Mar 2014, at 15:22, Sean Bright wrote: > >> On 3/14/2014 10:02 AM, Olle E. Johansson wrote: It would mean continuing to maintain Asterisk's pjproject fork until those changes were (hopefully) accepted upstream, releas

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

On 14 Mar 2014, at 15:22, Sean Bright wrote: > On 3/14/2014 10:02 AM, Olle E. Johansson wrote: >>> It would mean continuing to maintain Asterisk's pjproject fork until those >>> changes were (hopefully) accepted upstream, released, and then waiting for >>> the rpm/deb packages to catch up. No

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

On 3/14/2014 10:02 AM, Olle E. Johansson wrote: It would mean continuing to maintain Asterisk's pjproject fork until those changes were (hopefully) accepted upstream, released, and then waiting for the rpm/deb packages to catch up. Not to mention that someone would actually have to _do_ all of

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

On Fri, Mar 14, 2014 at 9:51 AM, Sean Bright wrote: > On 3/14/2014 2:41 AM, Olle E. Johansson wrote: > > > On 13 Mar 2014, at 22:13, Sean Bright wrote: > > On 3/13/2014 4:42 PM, Paul Belanger wrote: > > +1 with Dan. Comments aside on DNS functionality (I have opinions but > sitting this one out)

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

On 14 Mar 2014, at 14:51, Sean Bright wrote: > On 3/14/2014 2:41 AM, Olle E. Johansson wrote: >> >> On 13 Mar 2014, at 22:13, Sean Bright wrote: >> >>> On 3/13/2014 4:42 PM, Paul Belanger wrote: +1 with Dan. Comments aside on DNS functionality (I have opinions but sitting this one

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

On 3/14/2014 2:41 AM, Olle E. Johansson wrote: On 13 Mar 2014, at 22:13, Sean Bright > wrote: On 3/13/2014 4:42 PM, Paul Belanger wrote: +1 with Dan. Comments aside on DNS functionality (I have opinions but sitting this one out). Any functionality should be cha

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

On 13 Mar 2014, at 23:34, Matthew Jordan wrote: > (1) This is not "new" functionality - in the sense that we have not > written some amazing new functionality in either the core or a > res_pjsip* module and only want to use it one place. PJSIP itself > already provides DNS resolution. We just wa

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

On 13 Mar 2014, at 22:13, Sean Bright wrote: > On 3/13/2014 4:42 PM, Paul Belanger wrote: >> +1 with Dan. Comments aside on DNS functionality (I have opinions but >> sitting this one out). Any functionality should be channel agnostic. >> I too am a little concern'd that statement seems to have

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

On 13 Mar 2014, at 21:42, Paul Belanger wrote: > On Thu, Mar 13, 2014 at 4:15 PM, Dan Austin wrote: >> Matt wrote: >> >> Not including this change does not seem to buy us anything, save for some >> semblance of architectural purity. While I would love for there to be only >> one way to perform

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

> On March 13, 2014, 10:31 a.m., Olle E Johansson wrote: > > We really should NOT add this code. Instead, we should clean up the code so > > all of Asterisk use the same resolver and relies on the system > > configuration. Having a separate DNS resolver, maybe even using a separate > > DNS ser

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

On Thu, Mar 13, 2014 at 4:13 PM, Sean Bright wrote: > On 3/13/2014 4:42 PM, Paul Belanger wrote: > > +1 with Dan. Comments aside on DNS functionality (I have opinions but > sitting this one out). Any functionality should be channel agnostic. > I too am a little concern'd that statement seems to h

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

On 3/13/2014 4:42 PM, Paul Belanger wrote: +1 with Dan. Comments aside on DNS functionality (I have opinions but sitting this one out). Any functionality should be channel agnostic. I too am a little concern'd that statement seems to have changed. In order to make this "channel agnostic" you h

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

On Thu, Mar 13, 2014 at 4:15 PM, Dan Austin wrote: > Matt wrote: > > Not including this change does not seem to buy us anything, save for some > semblance of architectural purity. While I would love for there to be only > one way to perform DNS resolution, that feels like a long term goal - and >

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

Matt wrote: Not including this change does not seem to buy us anything, save for some semblance of architectural purity. While I would love for there to be only one way to perform DNS resolution, that feels like a long term goal – and sacrificing the practicality of delivering a feature that a

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

> On March 13, 2014, 4:31 p.m., Olle E Johansson wrote: > > We really should NOT add this code. Instead, we should clean up the code so > > all of Asterisk use the same resolver and relies on the system > > configuration. Having a separate DNS resolver, maybe even using a separate > > DNS serv

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

> On March 13, 2014, 10:31 a.m., Olle E Johansson wrote: > > We really should NOT add this code. Instead, we should clean up the code so > > all of Asterisk use the same resolver and relies on the system > > configuration. Having a separate DNS resolver, maybe even using a separate > > DNS ser

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3343/#review11170 --- We really should NOT add this code. Instead, we should clean up

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

- Original Message - > From: "Olle E. Johansson" > To: "Asterisk Developers Mailing List" > So we have yet another internal resolver? Is that a good thing? Why > are we not using the system resolver? > > We need to have some direction here. I think adding yet another DNS > resolver is b

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

On 13 Mar 2014, at 12:06, Joshua Colp wrote: > Olle E. Johansson wrote: >> >> On 13 Mar 2014, at 11:42, Joshua Colp > > wrote: >> >>> In case others are wondering as Olle was: >>> >>> PJLIB-Util (part of pjproject) provides a DNS client which can >>> optionall

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3343/#review11158 --- /branches/12/res/res_pjsip.c

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3343/#review11157 --- Ship it! Looks good to me. Now if only we could have it autom

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

Olle E. Johansson wrote: On 13 Mar 2014, at 11:42, Joshua Colp mailto:reviewbo...@asterisk.org>> wrote: In case others are wondering as Olle was: PJLIB-Util (part of pjproject) provides a DNS client which can optionally (but is highly suggested) to be used with PJSIP. It provides asynchronous

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

On 13 Mar 2014, at 11:42, Joshua Colp wrote: > In case others are wondering as Olle was: > > PJLIB-Util (part of pjproject) provides a DNS client which can optionally > (but is highly suggested) to be used with PJSIP. It provides asynchronous > DNS, SRV lookups, multiple record support, etc.

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

Olle E. Johansson wrote: On 12 Mar 2014, at 19:53, Joshua Colp mailto:reviewbo...@asterisk.org>> wrote: This change adds a configuration option for setting nameservers to be used by the PJSIP DNS client. If this option is not set then the system nameservers are retrieved and used instead. Thi

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3343/ --- (Updated March 13, 2014, 10:42 a.m.) Review request for Asterisk Developer

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

Olle E. Johansson wrote: On 12 Mar 2014, at 19:53, Joshua Colp mailto:reviewbo...@asterisk.org>> wrote: This change adds a configuration option for setting nameservers to be used by the PJSIP DNS client. If this option is not set then the system nameservers are retrieved and used instead. Thi

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

On 12 Mar 2014, at 19:53, Joshua Colp wrote: > This change adds a configuration option for setting nameservers to be used by > the PJSIP DNS client. If this option is not set then the system nameservers > are retrieved and used instead. > > This also allows the nameservers to be changed by do

[asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3343/ --- Review request for Asterisk Developers. Bugs: ASTERISK-23435 https://i