[asterisk-dev] ARI feedback

[David M. Lee]

On Oct 14, 2013, at 11:23 AM, Paul Belanger <paul.belanger at polybeacon.com> wrote:

> One thing I have noticed, it objects in the ARI don't seem to have a consistent behaviour when it comes to the 'id' parameter.
> 
> Most will default to 'id' as the primary key, however a few are just missing this or using a different field for it (name).  Seems trivial right now, but already building out a lib atop of ARI you can quickly see out you need to add additional layers of logic to check if ID exists or name, etc.

<snip/>

> I don't want to list them all, but I think you get the idea. Thoughts?

Given the naming convention I used, Sound's 'id' field probably should
have been 'name'.

The original intention was to model the objects as they exist in the
system. Some objects have an id, but no name (like Bridge). Some
objects have a name, but no id (like Recording), some have both (like
Channel), and some are just weird (like Endpoint).

I would think that the library would have specific code for the
different domain-objects, and that would be the place to handle type
differences (like named vs. id'ed). But there is a subtle difference
between the two: name carries meaning, whereas an id is opaque (you
should never care about the format or content of an id; just its
value).

While we could manufacture id's for objects that only carry names,
keeping the id's consistent would be a challenge. We'd have to store
them somewhere. And the manufactured id is more likely to change for
silly reasons, which would cause all sorts of headaches.

> Well, another issue I didn't mention is the mixture of camelCase and snake_case within the parameters.  At best, it is not consistent. Personally, I prefer snake_case, but that is a discussion point.  Morst importantly I think, is we pick one and stick to it.

The Swagger convention is camelCase, so the parameters are all
camelCase.

Asterisk, however, is snake_case, and the models are based on that.
That is an unfortunate inconsistency.

At the time, I was okay with it. Now I'm regretting the decision. But
since I would vote for camelCase, we might have a fight on our hands.


>>>> Since I'm typing, what are our chances of using enum fields for things
>>>> like state, bridge_type, technologies, etc. Personally, I'd like keep
>>>> string values out when possible and use an int / enum when I can.
>>>> 
>>> 
>>> Swagger supports enums, although I'll defer to David on all the pros and
>>> cons of using enums instead of more basic data types (for example, Python
>>> doesn't support enums until 3.4, so there may be some finicky aspects to
>>> adopting it).

The enums provided in Swagger are just restricted values on the string
type, which we use when appropriate. We avoid them when the list is
dynamic, like bridging technology.

It's still a string over the wire; a client library can use enums
based on the restricted values if it wants to.

>>>> And /sounds, I'd rather see id changed to name, since we are using the
>>>> filename for prompts.  This leaves us the ability to use a UUID if we
>>>> ever added it.
>>>> 
>>> 
>>> It isn't always a filename (or at least, I hope it won't always be soon!)
>>> We definitely want the ability for a URI to a remote resource to be able to
>>> be specified instead of a sound file located on the Asterisk system.
>>> Renaming it to filename here again makes that a bit more confusing, rather
>>> than clarifying.
>>> 
>> 
>> I'd agree in changing id to something else, but not name/filename etc -
>> maybe resource as the file would be a resource, a remote uri would be a
>> resource etc - these would still be identifiers

I'd be fine with 'name' since it's simply the name of the sound
(hello-world, beep, tt-monkeys) and doesn't imply filename at all.

>>>> Oh and lastly, can we please do /ari/v1/bridges, please, please,
>>>> please!  Or whatever or version number is. I really, really think we
>>>> need the namespace in the URL.
>>>> 
>>> 
>>> While I'm not sure we want the version number to be explicitly in the
>>> path, I do think we need versioning in ARI in some fashion, particularly if
>>> we start introducing breaking changes into the REST API mid-stream.
>>> 
>>> 
>> PLEASE do not do this, versioning shouldn't belong in a URL - as it doesn't
>> describe the resource. In my view, if you require a specific version of the
>> ARI, this should be done with headers - send in an "Accept-Version" header
>> which is a semver style "~2" for example
>> 
>> This way, you get the most up to date API if you don't ask for a specific
>> version.
>> 
>> The Restify project in npm does this (along with many many others) - take a
>> look at how they deal with requesting of versions here:
>> http://mcavage.me/node-restify/

I like the Accept-Version approach. I started writing a wiki page
about versioning, but then some distractions happened (like actually
getting the API to work :-), but I'll be sure to get back on that.
https://wiki.asterisk.org/wiki/x/WwCUAQ

-- 
David M. Lee
Digium, Inc. | Software Developer
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at:  www.digium.com  & www.asterisk.org