[asterisk-dev] [Code Review] 4101: Channel Originate/Continue via ARI support for labels in dialplan is incomplete

[greenfieldtech]

> On Nov. 5, 2014, 5:43 p.m., Joshua Colp wrote:
> > Matt brought it up that this is actually a backwards incompatible change - specifically changing priority into a string from an integer. What about having label as a separate argument that is optional? If present it's treated as a label and if not then the priority is used as normal. This allows labels to be used with no backwards incompatible changes and also makes it a bit more explicit from a developer side of what they want.
> 
> greenfieldtech wrote:
>     Wow, that was actually my initial idea almost 4 weeks ago - the only issue was that I was feeling it was truly unclean. If you look into Asterisk docs, priority and labels and normally mixed. For example, if you use Goto in dialplan - you can do Goto(context,exten,priority) or Goto(context,exten,label). I personally feel it should work exactly the same, I see no point in having two methodologies. In addition, what should be used if both are defined? which has the stronger priority here? I feel this will also bring much confusion into the mix. Again, I can easily revert my original code work, but in my opinion it is very much confusing.
>     
>     Anyone else would like to share their thought?
> 
> Scott Griepentrog wrote:
>     I agree that it would be easier to include this patch if it did not alter the existing API, only add an additional optional field.
>
> 
> Matt Jordan wrote:
>     This is one of those cases where - unfortunately - changing the type of the field would break existing clients and libraries.
>     
>     For example, if I were using Python and wanted to pass a priority into an origination, I might use the following:
>     
>     channels.originate(endpoint='PJSIP/bob', context='default', extension='1000', priority=1)
>     
>     That's different than:
>     
>     channels.originate(endpoint='PJSIP/bob', context='default', extension='1000', priority='1')
>     
>     I don't think we can change the type of the field without having to bump the ARI version to 2.0. That means using a different field, as was originally proposed.
>     
>     I do think that if we bump the major version to 2.x, this needs to be merged with the existing priority field. I've got a wiki page started for these kinds of things:
>     
>     https://wiki.asterisk.org/wiki/display/AST/Asterisk+API+Improvements

Ok, so let's agree on the functionality:

adding a new argument called "label". If present in the REST request, will take precedence to the priority argument - and will be used. 
If not defined, the "priority" argument will be used. 

I can accept the idea of breaking the existing API as a consideration. If we can agree on the above, I'll work on the patch to do that.


- greenfieldtech


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


On Nov. 5, 2014, 2:16 p.m., greenfieldtech wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4101/
> -----------------------------------------------------------
> 
> (Updated Nov. 5, 2014, 2:16 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24412
>     https://issues.asterisk.org/jira/browse/ASTERISK-24412
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This patch changes the current behavior of ARI, to allow channel originate/continue requests to be performed with labels as the priority, not only integer values.
> 
> 
> Diffs
> -----
> 
>   /trunk/rest-api/api-docs/channels.json 425359 
>   /trunk/res/res_ari_channels.c 425359 
>   /trunk/res/ari/resource_channels.c 425359 
>   /trunk/res/ari/resource_channels.h 425359 
> 
> Diff: https://reviewboard.asterisk.org/r/4101/diff/
> 
> 
> Testing
> -------
> 
> Testing was performed by testing the following scenarios:
> 1. Originating a call to a numeric priority - works
> 2. Originating a call to a null priority - works
> 3. Originating a call to a label - works
> 4. Continue a call to a label - not tested yet
> 
> 
> Thanks,
> 
> greenfieldtech
> 
>

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