Ashley Sanders has posted comments on this change. Change subject: stasis: set a channel variable on websocket disconnect error ......................................................................
Patch Set 2: (5 comments) https://gerrit.asterisk.org/#/c/18/2/tests/rest_api/applications/stasisstatus/monitor.py File tests/rest_api/applications/stasisstatus/monitor.py: Line 45: self.__ami.registerEvent('VarSet', self.__on_ami_varset) : self.__ami.registerEvent('UserEvent', self.__on_ami_user_event) : self.__ari_client.register_observers('on_stasisend', : self.__on_stasisend) : self.__ari_client.register_observers('on_stasisstart', : self.__on_stasisstart) : self.__ari_client.register_observers('on_channelvarset', : self.__on_channelvarset) > I'm not sure you need both AMI and ARI in this class. You are right - AMI is all that I need for monitoring the channel variable. https://gerrit.asterisk.org/#/c/18/2/tests/rest_api/applications/stasisstatus/observable_object.py File tests/rest_api/applications/stasisstatus/observable_object.py: Line 139: def __validate(self, **kwargs): I still need to apply the refactoring suggested by Mark Michelson in his review of patch 1. Essentially, he pointed out that the way that this method is being used (as a means to validate that a given event has been registered in the registrar) does not require the flexibility of allowing a variable set of arguments to be supplied. https://gerrit.asterisk.org/#/c/18/2/tests/rest_api/applications/stasisstatus/run-test File tests/rest_api/applications/stasisstatus/run-test: Line 87: def __on_channelstatechange(self, sender, message): This method has no usages and can safely be removed. https://gerrit.asterisk.org/#/c/18/2/tests/rest_api/applications/stasisstatus/test-config.yaml File tests/rest_api/applications/stasisstatus/test-config.yaml: Line 31: - ARI : - Stasis > Stasis is, in many ways, synonymous currently with ARI. Since we already do Noted. https://gerrit.asterisk.org/#/c/18/2/tests/rest_api/applications/stasisstatus/test_case.py File tests/rest_api/applications/stasisstatus/test_case.py: Line 37: TestCase.__init__(self) I noticed while reviewing Jonathan's review, https://gerrit.asterisk.org/#/c/28/, that pylint had been lying to me. TestCase is not an older-style class and all calls to the base type's methods should be invoked using the super keyword. There are several places in this file that need to be updated accordingly: ln 37 (here), ln 64, ln 126, and ln 142. -- 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: 2 Gerrit-Project: testsuite Gerrit-Branch: master Gerrit-Owner: Ashley Sanders <asand...@digium.com> Gerrit-Reviewer: Ashley Sanders <asand...@digium.com> Gerrit-Reviewer: Mark Michelson <mmichel...@digium.com> Gerrit-Reviewer: Matt Jordan <mjor...@digium.com> Gerrit-HasComments: Yes -- _____________________________________________________________________ -- 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