launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/gpg-timeline into lp:launchpad.

Commit message:
Add timeline information to all GPGME calls.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/gpg-timeline/+merge/340554

It can otherwise be rather hard to work out where time is being spent.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/gpg-timeline into lp:launchpad.

=== modified file 'lib/lp/services/gpg/handler.py'
--- lib/lp/services/gpg/handler.py	2018-01-26 22:18:38 +0000
+++ lib/lp/services/gpg/handler.py	2018-03-02 23:28:29 +0000
@@ -11,6 +11,7 @@
     ]
 
 import atexit
+from contextlib import contextmanager
 import httplib
 import os
 import shutil
@@ -66,6 +67,17 @@
 """
 
 
+@contextmanager
+def gpgme_timeline(name, detail):
+    request = get_current_browser_request()
+    timeline = get_request_timeline(request)
+    action = timeline.start("gpgme-%s" % name, detail, allow_nested=True)
+    try:
+        yield
+    finally:
+        action.finish()
+
+
 @implementer(IGPGHandler)
 class GPGHandler:
     """See IGPGHandler."""
@@ -178,16 +190,19 @@
             # store the content
             plain = StringIO(content)
             args = (sig, plain, None)
+            timeline_detail = "detached signature"
         else:
             # store clearsigned signature
             sig = StringIO(content)
             # writeable content
             plain = StringIO()
             args = (sig, None, plain)
+            timeline_detail = "clear signature"
 
         # process it
         try:
-            signatures = ctx.verify(*args)
+            with gpgme_timeline("verify", timeline_detail):
+                signatures = ctx.verify(*args)
         except gpgme.GpgmeError as e:
             error = GPGVerificationError(e.strerror)
             for attr in ("args", "code", "signatures", "source"):
@@ -238,7 +253,8 @@
         context = self._getContext()
 
         newkey = StringIO(content)
-        result = context.import_(newkey)
+        with gpgme_timeline("import", "new public key"):
+            result = context.import_(newkey)
 
         if len(result.imports) == 0:
             raise GPGKeyNotFoundError(content)
@@ -271,7 +287,8 @@
 
         context = self._getContext()
         newkey = StringIO(content)
-        import_result = context.import_(newkey)
+        with gpgme_timeline("import", "new secret key"):
+            import_result = context.import_(newkey)
 
         secret_imports = [
             fingerprint
@@ -303,8 +320,9 @@
         # Only 'utf-8' encoding is supported by gpgme.
         # See more information at:
         # http://pyme.sourceforge.net/doc/gpgme/Generating-Keys.html
-        result = context.genkey(
-            signing_only_param % {'name': name.encode('utf-8')})
+        with gpgme_timeline("genkey", name):
+            result = context.genkey(
+                signing_only_param % {'name': name.encode('utf-8')})
 
         # Right, it might seem paranoid to have this many assertions,
         # but we have to take key generation very seriously.
@@ -346,9 +364,10 @@
                              % key.fingerprint)
 
         # encrypt content
-        ctx.encrypt(
-            [removeSecurityProxy(key.key)], gpgme.ENCRYPT_ALWAYS_TRUST,
-            plain, cipher)
+        with gpgme_timeline("encrypt", key.fingerprint):
+            ctx.encrypt(
+                [removeSecurityProxy(key.key)], gpgme.ENCRYPT_ALWAYS_TRUST,
+                plain, cipher)
 
         return cipher.getvalue()
 
@@ -379,7 +398,8 @@
 
         # Sign the text.
         try:
-            context.sign(plaintext, signature, mode)
+            with gpgme_timeline("sign", key.fingerprint):
+                context.sign(plaintext, signature, mode)
         except gpgme.GpgmeError:
             return None
 
@@ -397,8 +417,10 @@
         if type(filter) == unicode:
             filter = filter.encode('utf-8')
 
-        for key in ctx.keylist(filter, secret):
-            yield PymeKey.newFromGpgmeKey(key)
+        with gpgme_timeline(
+                "keylist", "filter: %r, secret: %r" % (filter, secret)):
+            for key in ctx.keylist(filter, secret):
+                yield PymeKey.newFromGpgmeKey(key)
 
     def retrieveKey(self, fingerprint):
         """See IGPGHandler."""
@@ -414,7 +436,8 @@
             key = self.importPublicKey(pubkey)
             if fingerprint != key.fingerprint:
                 ctx = self._getContext()
-                ctx.delete(key.key)
+                with gpgme_timeline("delete", key.fingerprint):
+                    ctx.delete(key.key)
                 raise GPGKeyMismatchOnServer(fingerprint, key.fingerprint)
         return key
 
@@ -490,7 +513,8 @@
         request = get_current_browser_request()
         timeline = get_request_timeline(request)
         action = timeline.start(
-            'retrieving GPG key', 'Fingerprint: %s' % fingerprint)
+            'retrieving GPG key', 'Fingerprint: %s' % fingerprint,
+            allow_nested=True)
         try:
             return self._grabPage('get', fingerprint)
         # We record an OOPS for most errors: If the keyserver does not
@@ -568,7 +592,8 @@
         context = self._getContext()
         # retrive additional key information
         try:
-            key = context.get_key(fingerprint, False)
+            with gpgme_timeline("get-key", fingerprint):
+                key = context.get_key(fingerprint, False)
         except gpgme.GpgmeError:
             key = None
 
@@ -616,7 +641,8 @@
 
         context = self._getContext()
         keydata = StringIO()
-        context.export(self.fingerprint.encode('ascii'), keydata)
+        with gpgme_timeline("export", self.fingerprint):
+            context.export(self.fingerprint.encode('ascii'), keydata)
 
         return keydata.getvalue()