<p>Max <strong>merged</strong> this change.</p><p><a href="https://gerrit.osmocom.org/11953">View Change</a></p><div style="white-space:pre-wrap">Approvals:
  Pau Espin Pedrol: Looks good to me, approved
  Jenkins Builder: Verified

</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">ctrl2cgi: fix deferred callbacks<br><br>Previously handle_reply() was marked as deferred callback unlike soap.py<br>where it's synchronous function. This seems to be causing issues where<br>some of the callbacks are not yield properly. Let's move to the<br>known-to-work semantics of soap.py where async functions are limited to<br>Trap() class.<br><br>Change-Id: Ib2c28dd7f79cbd28d475de93750703659ddd18f1<br>Related: SYS#4399<br>---<br>M scripts/ctrl2cgi.py<br>1 file changed, 7 insertions(+), 18 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/scripts/ctrl2cgi.py b/scripts/ctrl2cgi.py</span><br><span>index 843aeb9..c566a7c 100755</span><br><span>--- a/scripts/ctrl2cgi.py</span><br><span>+++ b/scripts/ctrl2cgi.py</span><br><span>@@ -22,7 +22,7 @@</span><br><span>  */</span><br><span> """</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-__version__ = "0.0.2" # bump this on every non-trivial change</span><br><span style="color: hsl(120, 100%, 40%);">+__version__ = "0.0.3" # bump this on every non-trivial change</span><br><span> </span><br><span> from twisted.internet import defer, reactor</span><br><span> from osmopy.twisted_ipa import CTRL, IPAFactory, __version__ as twisted_ipa_version</span><br><span>@@ -40,25 +40,14 @@</span><br><span> assert V(twisted_ipa_version) > V('0.4')</span><br><span> </span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-@defer.inlineCallbacks</span><br><span> def handle_reply(f, log, resp):</span><br><span>     """</span><br><span>     Reply handler: process raw CGI server response, function f to run for each command</span><br><span>     """</span><br><span style="color: hsl(0, 100%, 40%);">-    #log.debug('HANDLE_REPLY: code=%r' % (resp.code))</span><br><span style="color: hsl(0, 100%, 40%);">-    #for key,val in resp.headers.getAllRawHeaders():</span><br><span style="color: hsl(0, 100%, 40%);">-    #    log.debug('HANDLE_REPLY: key=%r val=%r' % (key, val))</span><br><span style="color: hsl(0, 100%, 40%);">-    if resp.code != 200:</span><br><span style="color: hsl(0, 100%, 40%);">-        resp_body = yield resp.text()</span><br><span style="color: hsl(0, 100%, 40%);">-        log.critical('Received HTTP response %d: %s' % (resp.code, resp_body))</span><br><span style="color: hsl(0, 100%, 40%);">-        return</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-    parsed = yield resp.json()</span><br><span style="color: hsl(0, 100%, 40%);">-    #log.debug("RESPONSE: %r" % (parsed))</span><br><span style="color: hsl(0, 100%, 40%);">-    bsc_id = parsed.get('commands')[0].split()[0].split('.')[3] # we expect 1st command to have net.0.bsc.666.bts.2.trx.1 location prefix format</span><br><span style="color: hsl(0, 100%, 40%);">-    log.info("Received CGI response for BSC %s with %d commands, error status: %s" % (bsc_id, len(parsed.get('commands')), parsed.get('error')))</span><br><span style="color: hsl(0, 100%, 40%);">-    log.debug("BSC %s commands: %r" % (bsc_id, parsed.get('commands')))</span><br><span style="color: hsl(0, 100%, 40%);">-    for t in parsed.get('commands'): # Process commands format</span><br><span style="color: hsl(120, 100%, 40%);">+    decoded = json.loads(resp.decode('utf-8'))</span><br><span style="color: hsl(120, 100%, 40%);">+    bsc_id = decoded.get('commands')[0].split()[0].split('.')[3] # we expect 1st command to have net.0.bsc.666.bts.2.trx.1 location prefix format</span><br><span style="color: hsl(120, 100%, 40%);">+    log.debug("BSC %s commands: %r" % (bsc_id, decoded.get('commands')))</span><br><span style="color: hsl(120, 100%, 40%);">+    for t in decoded.get('commands'): # Process commands format</span><br><span>         (_, m) = Ctrl().cmd(*t.split())</span><br><span>         f(m)</span><br><span> </span><br><span>@@ -116,8 +105,8 @@</span><br><span>         params = make_params(bsc, data)</span><br><span>         self.factory.log.debug('location-state@%s.%s.%s.%s (%s) => %s' % (net, bsc, bts, trx, params['time_stamp'], data))</span><br><span>         params['h'] = gen_hash(params, self.factory.secret_key)</span><br><span style="color: hsl(0, 100%, 40%);">-        d = post(self.factory.location, None, params=params)</span><br><span style="color: hsl(0, 100%, 40%);">-        d.addCallback(partial(handle_reply, self.transport.write, self.factory.log)) # treq's collect helper is handy to get all reply content at once using closure on ctx</span><br><span style="color: hsl(120, 100%, 40%);">+        d = post(self.factory.location, params)</span><br><span style="color: hsl(120, 100%, 40%);">+        d.addCallback(collect, partial(handle_reply, self.transport.write, self.factory.log)) # treq's collect helper is handy to get all reply content at once</span><br><span>         d.addErrback(lambda e, bsc: self.factory.log.critical("HTTP POST error %s while trying to register BSC %s on %s" % (e, bsc, self.factory.location)), bsc) # handle HTTP errors</span><br><span>         # Ensure that we run only limited number of requests in parallel:</span><br><span>         yield self.factory.semaphore.acquire()</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/11953">change 11953</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/11953"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: python/osmo-python-tests </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: Ib2c28dd7f79cbd28d475de93750703659ddd18f1 </div>
<div style="display:none"> Gerrit-Change-Number: 11953 </div>
<div style="display:none"> Gerrit-PatchSet: 4 </div>
<div style="display:none"> Gerrit-Owner: Max <msuraev@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: Max <msuraev@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Pau Espin Pedrol <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: daniel <dwillmann@sysmocom.de> </div>