Ashley Sanders has posted comments on this change. Change subject: stasis: set a channel variable on websocket disconnect error ......................................................................
Patch Set 2: (5 comments) Addressed Matt Jordan's review feedback. https://gerrit.asterisk.org/#/c/18/2/tests/rest_api/applications/stasisstatus/ari_client.py File tests/rest_api/applications/stasisstatus/ari_client.py: Line 71: while not self.clean: : LOGGER.debug('{0} I\'m not so fresh so clean.'.format(self)) : time.sleep(1) > There's a few problems with this particular approach. I opted for option #2. In the case that we get stuck in a loop, the reactor will timeout, causing the test to fail. I think this is the preferred behavior in that the reactor is handling the timing and it will assuredly blow up quickly if something gets stuck during processing. Line 175: def on_channelstatechange(self, message): While going through Matt's review feedback, I realized the only place this was being used was from a method that was never called in BabsTestScenario. Line 270: resource -- The resource to use to construct the endpoint : for the ARI request. (optional) : (default None.) If no value is provided, : resource will automatically be generated in : the form: 'LOCAL/<app_name>@Acme'. > Good catch. Yes, I 100% agree with you. This should be refactored to live i Also, since it is not mandatory for a client to pass an app parameter to ARI, I added a check for that argument and if it is None, it does not include it in the post to ARI. https://gerrit.asterisk.org/#/c/18/2/tests/rest_api/applications/stasisstatus/configs/ast1/extensions.conf File tests/rest_api/applications/stasisstatus/configs/ast1/extensions.conf: Line 4: I am missing the Babs extension in this file. 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 33: - Application : - STASISSTATUS > I'd remove these two tags. Noted. I reviewed all the current tags in the testsuite and the only ones that are applicable to this are ARI and Application. I also added a tag for the jira issue. -- 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
