Re: [asterisk-dev] Possible change to the AMI PJSIPShowRegistrationsInbound command

[George Joseph]

On Wed, Dec 7, 2016 at 8:34 AM, George Joseph  wrote:

>
>
> On Wed, Dec 7, 2016 at 8:16 AM, Matthew Jordan  wrote:
>
>>
>>
>> On Tue, Dec 6, 2016 at 4:27 PM, Matt Fredrickson 
>> wrote:
>>
>>> On Tue, Dec 6, 2016 at 12:30 PM, Joshua Colp  wrote:
>>> > On Tue, Dec 6, 2016, at 10:43 AM, George Joseph wrote:
>>> >> We just discovered that the PJSIPShowRegistrationsInbound command
>>> really
>>> >> only dumps all aors regardless of whether there's an inbound
>>> registration
>>> >> associated with it or not.  Fixing this would mean a change to what
>>> this
>>> >> command returns so I'm trying to get some feedback.  There are 2
>>> solution
>>> >> alternatives...
>>> >>
>>> >> 1.  We could replace the current InboundRegistrationDetail event
>>> (which
>>> >> isn't even registered) with a ContactStatusDetail event.  Obviously
>>> this
>>> >> is
>>> >> a change for anyone who uses this command.
>>> >>
>>> >> 2.  We could send a ContactStatusDetail event along with the existing
>>> >> InboundRegistrationDetail event.  This would double the number of
>>> events
>>> >> sent and therefore increase the total data sent.
>>> >>
>>> >> Honestly I'm not sure how this command was ever useful to anybody so
>>> I'm
>>> >> leaning towards option 1 but we need feedback.
>>> >>
>>> >> This would be a change to 13, 14 and master.
>>> >
>>> > Since it was not really useful I'm okay with 1.
>>>
>>> Just as something to consider:
>>>
>>> If we make breaking changes to AMI, this would require a major number
>>> version bump, right?
>>>
>>>
>> Yes, it would.
>>
>
> AMI versions bumped:
> Branch 13: 3.0.0
> Branch 14 and master: 4.0.0
>


OK, We backtracked on this:

A new command was added PJSIPShowRegistrationInboundContactStatuses instead
of modifying the old one.  No version bumps of any kind
-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] Possible change to the AMI PJSIPShowRegistrationsInbound command

[George Joseph]

On Wed, Dec 7, 2016 at 8:16 AM, Matthew Jordan  wrote:

>
>
> On Tue, Dec 6, 2016 at 4:27 PM, Matt Fredrickson 
> wrote:
>
>> On Tue, Dec 6, 2016 at 12:30 PM, Joshua Colp  wrote:
>> > On Tue, Dec 6, 2016, at 10:43 AM, George Joseph wrote:
>> >> We just discovered that the PJSIPShowRegistrationsInbound command
>> really
>> >> only dumps all aors regardless of whether there's an inbound
>> registration
>> >> associated with it or not.  Fixing this would mean a change to what
>> this
>> >> command returns so I'm trying to get some feedback.  There are 2
>> solution
>> >> alternatives...
>> >>
>> >> 1.  We could replace the current InboundRegistrationDetail event (which
>> >> isn't even registered) with a ContactStatusDetail event.  Obviously
>> this
>> >> is
>> >> a change for anyone who uses this command.
>> >>
>> >> 2.  We could send a ContactStatusDetail event along with the existing
>> >> InboundRegistrationDetail event.  This would double the number of
>> events
>> >> sent and therefore increase the total data sent.
>> >>
>> >> Honestly I'm not sure how this command was ever useful to anybody so
>> I'm
>> >> leaning towards option 1 but we need feedback.
>> >>
>> >> This would be a change to 13, 14 and master.
>> >
>> > Since it was not really useful I'm okay with 1.
>>
>> Just as something to consider:
>>
>> If we make breaking changes to AMI, this would require a major number
>> version bump, right?
>>
>>
> Yes, it would.
>

AMI versions bumped:
Branch 13: 3.0.0
Branch 14 and master: 4.0.0


>
> --
> Matthew Jordan
> Digium, Inc. | CTO
> 445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
> Check us out at: http://digium.com & http://asterisk.org
>
> --
> _
> -- Bandwidth and Colocation Provided by http://www.api-digital.com --
>
> asterisk-dev mailing list
> To UNSUBSCRIBE or update options visit:
>http://lists.digium.com/mailman/listinfo/asterisk-dev
>



-- 
George Joseph
Digium, Inc. | Software Developer
445 Jan Davis Drive NW - Huntsville, AL 35806 - US
Check us out at: www.digium.com & www.asterisk.org
-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] Possible change to the AMI PJSIPShowRegistrationsInbound command

[Matthew Jordan]

On Tue, Dec 6, 2016 at 4:27 PM, Matt Fredrickson  wrote:

> On Tue, Dec 6, 2016 at 12:30 PM, Joshua Colp  wrote:
> > On Tue, Dec 6, 2016, at 10:43 AM, George Joseph wrote:
> >> We just discovered that the PJSIPShowRegistrationsInbound command really
> >> only dumps all aors regardless of whether there's an inbound
> registration
> >> associated with it or not.  Fixing this would mean a change to what this
> >> command returns so I'm trying to get some feedback.  There are 2
> solution
> >> alternatives...
> >>
> >> 1.  We could replace the current InboundRegistrationDetail event (which
> >> isn't even registered) with a ContactStatusDetail event.  Obviously this
> >> is
> >> a change for anyone who uses this command.
> >>
> >> 2.  We could send a ContactStatusDetail event along with the existing
> >> InboundRegistrationDetail event.  This would double the number of events
> >> sent and therefore increase the total data sent.
> >>
> >> Honestly I'm not sure how this command was ever useful to anybody so I'm
> >> leaning towards option 1 but we need feedback.
> >>
> >> This would be a change to 13, 14 and master.
> >
> > Since it was not really useful I'm okay with 1.
>
> Just as something to consider:
>
> If we make breaking changes to AMI, this would require a major number
> version bump, right?
>
>
Yes, it would.

-- 
Matthew Jordan
Digium, Inc. | CTO
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at: http://digium.com & http://asterisk.org
-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] Possible change to the AMI PJSIPShowRegistrationsInbound command

[Matt Fredrickson]

On Tue, Dec 6, 2016 at 12:30 PM, Joshua Colp  wrote:
> On Tue, Dec 6, 2016, at 10:43 AM, George Joseph wrote:
>> We just discovered that the PJSIPShowRegistrationsInbound command really
>> only dumps all aors regardless of whether there's an inbound registration
>> associated with it or not.  Fixing this would mean a change to what this
>> command returns so I'm trying to get some feedback.  There are 2 solution
>> alternatives...
>>
>> 1.  We could replace the current InboundRegistrationDetail event (which
>> isn't even registered) with a ContactStatusDetail event.  Obviously this
>> is
>> a change for anyone who uses this command.
>>
>> 2.  We could send a ContactStatusDetail event along with the existing
>> InboundRegistrationDetail event.  This would double the number of events
>> sent and therefore increase the total data sent.
>>
>> Honestly I'm not sure how this command was ever useful to anybody so I'm
>> leaning towards option 1 but we need feedback.
>>
>> This would be a change to 13, 14 and master.
>
> Since it was not really useful I'm okay with 1.

Just as something to consider:

If we make breaking changes to AMI, this would require a major number
version bump, right?

-- 
Matthew Fredrickson
Digium, Inc. | Engineering Manager
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] Possible change to the AMI PJSIPShowRegistrationsInbound command

[Joshua Colp]

On Tue, Dec 6, 2016, at 10:43 AM, George Joseph wrote:
> We just discovered that the PJSIPShowRegistrationsInbound command really
> only dumps all aors regardless of whether there's an inbound registration
> associated with it or not.  Fixing this would mean a change to what this
> command returns so I'm trying to get some feedback.  There are 2 solution
> alternatives...
> 
> 1.  We could replace the current InboundRegistrationDetail event (which
> isn't even registered) with a ContactStatusDetail event.  Obviously this
> is
> a change for anyone who uses this command.
> 
> 2.  We could send a ContactStatusDetail event along with the existing
> InboundRegistrationDetail event.  This would double the number of events
> sent and therefore increase the total data sent.
> 
> Honestly I'm not sure how this command was ever useful to anybody so I'm
> leaning towards option 1 but we need feedback.
> 
> This would be a change to 13, 14 and master.

Since it was not really useful I'm okay with 1.

-- 
Joshua Colp
Digium, Inc. | Senior Software Developer
445 Jan Davis Drive NW - Huntsville, AL 35806 - US
Check us out at: www.digium.com & www.asterisk.org

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] Possible change to the AMI PJSIPShowRegistrationsInbound command

[George Joseph]

On Tue, Dec 6, 2016 at 10:25 AM, Dan Jenkins 
wrote:

>
>
>> I'd go with Option 1 - although it feels wrong breaking something
> (regardless as to whether it was already broken) within a minor/patch
> release.
>
> In theory people should be reading release notes but I fall foul of not
> reading release notes for breakages within a minor/patch update - because
> in theory it shouldn't break anything... But we all know this does indeed
> happen. I would argue the current behaviour is broken and so you are fixing
> the broken behaviour - if people are relying on the data in this command
> then they're relying on incorrect data anyway.
>

I like your reasoning.


>
> --
> _
> -- Bandwidth and Colocation Provided by http://www.api-digital.com --
>
> asterisk-dev mailing list
> To UNSUBSCRIBE or update options visit:
>http://lists.digium.com/mailman/listinfo/asterisk-dev
>



-- 
George Joseph
Digium, Inc. | Software Developer
445 Jan Davis Drive NW - Huntsville, AL 35806 - US
Check us out at: www.digium.com & www.asterisk.org
-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] Possible change to the AMI PJSIPShowRegistrationsInbound command

[Dan Jenkins]

On Tue, Dec 6, 2016 at 2:43 PM, George Joseph  wrote:

> We just discovered that the PJSIPShowRegistrationsInbound command really
> only dumps all aors regardless of whether there's an inbound registration
> associated with it or not.  Fixing this would mean a change to what this
> command returns so I'm trying to get some feedback.  There are 2 solution
> alternatives...
>
> 1.  We could replace the current InboundRegistrationDetail event (which
> isn't even registered) with a ContactStatusDetail event.  Obviously this is
> a change for anyone who uses this command.
>
> 2.  We could send a ContactStatusDetail event along with the existing
> InboundRegistrationDetail event.  This would double the number of events
> sent and therefore increase the total data sent.
>
> Honestly I'm not sure how this command was ever useful to anybody so I'm
> leaning towards option 1 but we need feedback.
>
> This would be a change to 13, 14 and master.
>
>
> --
> George Joseph
> Digium, Inc. | Software Developer
> 445 Jan Davis Drive NW - Huntsville, AL 35806 - US
> Check us out at: www.digium.com & www.asterisk.org
>
>
> --
> _
> -- Bandwidth and Colocation Provided by http://www.api-digital.com --
>
> asterisk-dev mailing list
> To UNSUBSCRIBE or update options visit:
>http://lists.digium.com/mailman/listinfo/asterisk-dev
>

I'd go with Option 1 - although it feels wrong breaking something
(regardless as to whether it was already broken) within a minor/patch
release.

In theory people should be reading release notes but I fall foul of not
reading release notes for breakages within a minor/patch update - because
in theory it shouldn't break anything... But we all know this does indeed
happen. I would argue the current behaviour is broken and so you are fixing
the broken behaviour - if people are relying on the data in this command
then they're relying on incorrect data anyway.
-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Possible change to the AMI PJSIPShowRegistrationsInbound command

[George Joseph]

We just discovered that the PJSIPShowRegistrationsInbound command really
only dumps all aors regardless of whether there's an inbound registration
associated with it or not.  Fixing this would mean a change to what this
command returns so I'm trying to get some feedback.  There are 2 solution
alternatives...

1.  We could replace the current InboundRegistrationDetail event (which
isn't even registered) with a ContactStatusDetail event.  Obviously this is
a change for anyone who uses this command.

2.  We could send a ContactStatusDetail event along with the existing
InboundRegistrationDetail event.  This would double the number of events
sent and therefore increase the total data sent.

Honestly I'm not sure how this command was ever useful to anybody so I'm
leaning towards option 1 but we need feedback.

This would be a change to 13, 14 and master.


-- 
George Joseph
Digium, Inc. | Software Developer
445 Jan Davis Drive NW - Huntsville, AL 35806 - US
Check us out at: www.digium.com & www.asterisk.org
-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev