[PATCH] openbsc[master]: twisted_ipa.py: make debug logging more robust

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Max gerrit-no-reply at lists.osmocom.org
Mon Mar 20 11:58:15 UTC 2017


Review at  https://gerrit.osmocom.org/2133

twisted_ipa.py: make debug logging more robust

Do not print anything to stdout directly - use proper logger object
instead: either the one supplied by IPAFactory user or default to NO-OP
NullHandler logger. While at it, also move version string to comply with
PEP8.

Change-Id: Ic3417095a6e8848f0acabb46a9e64c0197b736e2
Related: SYS#3028
---
M openbsc/contrib/twisted_ipa.py
1 file changed, 30 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/33/2133/1

diff --git a/openbsc/contrib/twisted_ipa.py b/openbsc/contrib/twisted_ipa.py
index 8d0ef9f..f688aef 100755
--- a/openbsc/contrib/twisted_ipa.py
+++ b/openbsc/contrib/twisted_ipa.py
@@ -22,11 +22,13 @@
  */
 """
 
+__version__ = "v0.4" # bump this on every non-trivial change
+
 from ipa import Ctrl, IPA
 from twisted.internet.protocol import ReconnectingClientFactory
 from twisted.internet import reactor
 from twisted.protocols import basic
-import argparse
+import argparse, logging, logging.handlers
 
 class IPACommon(basic.Int16StringReceiver):
     """
@@ -37,8 +39,7 @@
         """
         Debug print helper
         """
-        if self.factory.debug:
-            print(line)
+        self.factory.log.debug(line)
 
     def osmo_CTRL(self, data):
         """
@@ -257,7 +258,7 @@
         Keep reconnection logic working by calling routine from CCM
         Initiate CCM upon connection
         """
-        print('IPA server connection made!')
+        self.dbg('IPA server connection made!')
         super(IPAServer, self).connectionMade()
         self.transport.write(IPA().id_get())
 
@@ -273,7 +274,7 @@
         Send TRAP upon connection
         Note: we can't use sendString() because of it's incompatibility with IPA interpretation of length prefix
         """
-        print('CTRL server connection made!')
+        self.dbg('CTRL server connection made!')
         super(CtrlServer, self).connectionMade()
         self.transport.write(Ctrl().trap('LOL', 'what'))
         self.transport.write(Ctrl().trap('rulez', 'XXX'))
@@ -285,14 +286,14 @@
         """
         CTRL SET command: always succeed
         """
-        print('SET [%s] %s' % (op_id, v))
+        self.dbg('SET [%s] %s' % (op_id, v))
         self.reply('SET_REPLY %s %s' % (op_id, v))
 
     def ctrl_GET(self, data, op_id, v):
         """
         CTRL GET command: always fail
         """
-        print('GET [%s] %s' % (op_id, v))
+        self.dbg('GET [%s] %s' % (op_id, v))
         self.reply('ERROR %s No variable found' % op_id)
 
 
@@ -302,37 +303,39 @@
     Note: so far we do not really need separate Factory for acting as a server due to protocol simplicity
     """
     protocol = IPACommon
-    debug = False
+    log = None
     ccm_id = IPA().identity(unit=b'1515/0/1', mac=b'b0:0b:fa:ce:de:ad:be:ef', utype=b'sysmoBTS', name=b'StingRay', location=b'hell', sw=IPA.version.encode('utf-8'))
 
-    def __init__(self, proto=None, debug=False, ccm_id=None):
+    def __init__(self, proto=None, log=None, ccm_id=None):
         if proto:
             self.protocol = proto
-        if debug:
-            self.debug = debug
         if ccm_id:
             self.ccm_id = ccm_id
+        if log:
+            self.log = log
+        else:
+            self.log = logging.getLogger('IPAFactory')
+            log.setLevel(logging.CRITICAL)
+            log.addHandler(logging.NullHandler)
 
     def clientConnectionFailed(self, connector, reason):
         """
         Only necessary for as debugging aid - if we can somehow set parent's class noisy attribute then we can omit this method
         """
-        if self.debug:
-            print('IPAFactory connection failed:', reason.getErrorMessage())
+        self.log.debug('IPAFactory connection failed:', reason.getErrorMessage())
         ReconnectingClientFactory.clientConnectionFailed(self, connector, reason)
 
     def clientConnectionLost(self, connector, reason):
         """
         Only necessary for as debugging aid - if we can somehow set parent's class noisy attribute then we can omit this method
         """
-        if self.debug:
-            print('IPAFactory connection lost:', reason.getErrorMessage())
+        self.log.debug('IPAFactory connection lost:', reason.getErrorMessage())
         ReconnectingClientFactory.clientConnectionLost(self, connector, reason)
 
 
 if __name__ == '__main__':
     p = argparse.ArgumentParser("Twisted IPA (module v%s) app" % IPA.version)
-    p.add_argument('-v', '--version', action='version', version='%(prog)s v0.3')
+    p.add_argument('-v', '--version', action='version', version="%(prog)s " + __version__)
     p.add_argument('-p', '--port', type=int, default=4250, help="Port to use for CTRL interface")
     p.add_argument('-d', '--host', default='localhost', help="Adress to use for CTRL interface")
     cs = p.add_mutually_exclusive_group()
@@ -343,29 +346,34 @@
     ic.add_argument("--ctrl", action='store_true', help="use CTRL protocol")
     args = p.parse_args()
     test = False
+
+    log = logging.getLogger('TwistedIPA')
+    log.setLevel(logging.DEBUG)
+    log.addHandler(logging.StreamHandler(sys.stdout))
+
     if args.ctrl:
         if args.client:
             # Start osmo-bsc to receive TRAP messages when osmo-bts-* connects to it
             print('CTRL client, connecting to %s:%d' % (args.host, args.port))
-            reactor.connectTCP(args.host, args.port, IPAFactory(CTRL, debug=True))
+            reactor.connectTCP(args.host, args.port, IPAFactory(CTRL, log))
             test = True
         if args.server:
             # Use bsc_control.py to issue set/get commands
             print('CTRL server, listening on port %d' % args.port)
-            reactor.listenTCP(args.port, IPAFactory(CtrlServer, debug=True))
+            reactor.listenTCP(args.port, IPAFactory(CtrlServer, log))
             test = True
     if args.ipa:
         if args.client:
             # Start osmo-nitb which would initiate A-bis/IP session
             print('IPA client, connecting to %s ports %d and %d' % (args.host, IPA.TCP_PORT_OML, IPA.TCP_PORT_RSL))
-            reactor.connectTCP(args.host, IPA.TCP_PORT_OML, IPAFactory(CCM, debug=True))
-            reactor.connectTCP(args.host, IPA.TCP_PORT_RSL, IPAFactory(CCM, debug=True))
+            reactor.connectTCP(args.host, IPA.TCP_PORT_OML, IPAFactory(CCM, log))
+            reactor.connectTCP(args.host, IPA.TCP_PORT_RSL, IPAFactory(CCM, log))
             test = True
         if args.server:
             # Start osmo-bts-* which would attempt to connect to us
             print('IPA server, listening on ports %d and %d' % (IPA.TCP_PORT_OML, IPA.TCP_PORT_RSL))
-            reactor.listenTCP(IPA.TCP_PORT_RSL, IPAFactory(IPAServer, debug=True))
-            reactor.listenTCP(IPA.TCP_PORT_OML, IPAFactory(IPAServer, debug=True))
+            reactor.listenTCP(IPA.TCP_PORT_RSL, IPAFactory(IPAServer, log))
+            reactor.listenTCP(IPA.TCP_PORT_OML, IPAFactory(IPAServer, log))
             test = True
     if test:
         reactor.run()

-- 
To view, visit https://gerrit.osmocom.org/2133
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic3417095a6e8848f0acabb46a9e64c0197b736e2
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>



More information about the gerrit-log mailing list