[asterisk-dev] Change in testsuite[master]: stasis: set a channel variable on websocket disconnect error

Mark Michelson (Code Review) asteriskteam at digium.com
Wed Apr 8 11:42:34 CDT 2015


Mark Michelson has posted comments on this change.

Change subject: stasis: set a channel variable on websocket disconnect error
......................................................................


Patch Set 5: Code-Review+1

(2 comments)

It's looking good by me!

https://gerrit.asterisk.org/#/c/18/5/tests/rest_api/applications/stasisstatus/ari_client.py
File tests/rest_api/applications/stasisstatus/ari_client.py:

Line 365:            if run is 0:
        :                 LOGGER.debug(msg + 'Tearing down active connections.')
        :                 self.__delete_all_channels()
        :                 reactor.callLater(2, wait_for_it, deferred, 1)
        :             elif run is 1:
        :                 if len(self.__channels) > 0:
        :                     msg += 'Waiting for channels to be destroyed.'
        :                     LOGGER.debug(msg)
        :                     reactor.callLater(2, wait_for_it, deferred, 1)
        :                 reactor.callLater(2, wait_for_it, deferred, 2)
        :             elif run is 2:
        :                 LOGGER.debug(msg + 'Disconnecting web socket.')
        :                 self.__ari = None
        :                 self.__factory = None
        :                 self.disconnect_websocket()
        :                 reactor.callLater(2, wait_for_it, deferred, 3)
        :             elif run is 3:
This is another of those cases where I'm not 100% sure on this, but I think the use of the 'is' operator in these comparisons is iffy. The 'is' operator returns true if the operands on either side refer to the same object, not if their values are equal. I'm honestly not sure what the behavior is when an integer literal is used as an operand, though. If I were writing this, though, I'd default to the safe bet and use the '==' operator for these comparisons instead.


https://gerrit.asterisk.org/#/c/18/5/tests/rest_api/applications/stasisstatus/configs/ast1/sip.conf
File tests/rest_api/applications/stasisstatus/configs/ast1/sip.conf:

Line 9: [acme](!)
> I am not sure that this template or the sherman endpoint is being used any 
In fact, I don't think SIP is being used at all for your tests, so you should be able to just scrap this conf file entirely.


-- 
To view, visit https://gerrit.asterisk.org/18
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0f7dadfd429bd30e9f07a531f47884d8c923fc13
Gerrit-PatchSet: 5
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-HasComments: Yes



More information about the asterisk-dev mailing list