[asterisk-dev] [Code Review] 2428: Add channel event distribution to app_stasis

[David Lee]

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


Just a quick review for some of the obvious things I found. For the most part, I like what you've done here!

After it's in trunk, I may work with it to generalize to to handle all of our models, not just the events.


trunk/rest-api-templates/asterisk_processor.py
<https://reviewboard.asterisk.org/r/2428/#comment16289>

    Unused



trunk/rest-api-templates/asterisk_processor.py
<https://reviewboard.asterisk.org/r/2428/#comment16288>

    I don't like modifying the existing models attribute. You should create a new events attribute instead.



trunk/rest-api-templates/swagger_model.py
<https://reviewboard.asterisk.org/r/2428/#comment16282>

    This should be self.description instead of self.notes.



trunk/rest-api-templates/swagger_model.py
<https://reviewboard.asterisk.org/r/2428/#comment16284>

    Maybe has_events would be a better name?



trunk/rest-api-templates/swagger_model.py
<https://reviewboard.asterisk.org/r/2428/#comment16285>

    Since that's not a part of the Swagger specification, this should be done in the AsteriskProcessor.



trunk/rest-api/api-docs/events.json
<https://reviewboard.asterisk.org/r/2428/#comment16286>

    You lost the bridge events there.



trunk/rest-api/api-docs/events.json
<https://reviewboard.asterisk.org/r/2428/#comment16290>

    JSON fields are snake_case. I can see why you'd want to match the Manager field name, keeping it consistent would be better.



trunk/rest-api/api-docs/events.json
<https://reviewboard.asterisk.org/r/2428/#comment16291>

    Dashes might especially cause problems mapping fields in some languages.



trunk/rest-api/api-docs/events.json
<https://reviewboard.asterisk.org/r/2428/#comment16292>

    snake_case



trunk/rest-api/api-docs/events.json
<https://reviewboard.asterisk.org/r/2428/#comment16293>

    snake_case



trunk/rest-api/api-docs/events.json
<https://reviewboard.asterisk.org/r/2428/#comment16294>

    snake_case



trunk/rest-api/api-docs/events.json
<https://reviewboard.asterisk.org/r/2428/#comment16295>

    snake_case



trunk/rest-api/api-docs/events.json
<https://reviewboard.asterisk.org/r/2428/#comment16296>

    snake_case



trunk/rest-api/api-docs/events.json
<https://reviewboard.asterisk.org/r/2428/#comment16297>

    This is especially weird, since the body is a "name: value\r\n" formatted string. Maybe we can be a little bit nicer and parse the provided user data into a JSON object? Thoughts?



trunk/rest-api/api-docs/events.json
<https://reviewboard.asterisk.org/r/2428/#comment16298>

    JSON has booleans.


- David Lee


On April 30, 2013, 9:03 a.m., opticron wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2428/
> -----------------------------------------------------------
> 
> (Updated April 30, 2013, 9:03 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-21180
>     https://issues.asterisk.org/jira/browse/ASTERISK-21180
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Add publication of AMI-equivalent channel event messages to users of app_stasis using JSON blobs.
> 
> 
> Diffs
> -----
> 
>   trunk/include/asterisk/astobj2.h 386934 
>   trunk/include/asterisk/stasis_channels.h 386934 
>   trunk/main/astobj2.c 386934 
>   trunk/main/manager_channels.c 386934 
>   trunk/main/stasis_channels.c 386934 
>   trunk/res/res_stasis.c 386934 
>   trunk/res/res_stasis_http_events.c 386934 
>   trunk/res/res_stasis_websocket.c 386934 
>   trunk/res/stasis_http/resource_events.h 386934 
>   trunk/res/stasis_http/resource_recordings.h 386934 
>   trunk/res/stasis_http/resource_sounds.h 386934 
>   trunk/rest-api-templates/asterisk_processor.py 386934 
>   trunk/rest-api-templates/event_function_decl.mustache PRE-CREATION 
>   trunk/rest-api-templates/res_stasis_http_resource.c.mustache 386934 
>   trunk/rest-api-templates/stasis_http_resource.h.mustache 386934 
>   trunk/rest-api-templates/swagger_model.py 386934 
>   trunk/rest-api/api-docs/events.json 386934 
> 
> Diff: https://reviewboard.asterisk.org/r/2428/diff/
> 
> 
> Testing
> -------
> 
> Tested with a hacked together app_stasis app which dumps incoming messages.
> 
> 
> Thanks,
> 
> opticron
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130502/ab4899dc/attachment-0001.htm>