launchpad-reviewers team mailing list archive

[Martin Pool <martinpool@xxxxxxxxx>]

Martin Pool has proposed merging lp:~mbp/launchpad/uniqueid-clue into lp:launchpad.

Requested reviews:
  Andrew Bennetts (spiv)
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~mbp/launchpad/uniqueid-clue/+merge/61153

Here's an idea from spiv that came up in the context of debugging a somewhat mysterious failure in <https://code.launchpad.net/~mbp/launchpad/778437-sprb-spam/+merge/60360>.  In that case, what ended up happening was the buildmaster tried to connect to random non-existent slaves, but the actual message (even with twisted debug on) was just

<DelayedCall 0xa756ab8 [59.9995620251s] called=0 cancelled=0 ThreadedResolver._cleanup('generic-string231767', <Deferred at 0xa756d40>)>

which is not very illuminating.

This inserts a string about the context of the origin of the string into the string so that if it crops up in a cryptic error, you will have more of a hint as to what kind of a thing it is.

I kicked off an ec2 run.

It's possible this will make things noticeably slower, in which case:
 1- well, maybe it won't, which would be nice  ;-)
 2- we could make it optionally turn on from a global or environment variable, though then it won't help people who don't know about it, or for errors that happen intermittently or in ec2
 3- we could make the commonly-hit calls just specify a string, as i have for a few here
 4- something else

One other related thing is that I noticed but have not addressed is that the sequence numbers go off a per-thread random series, which means they will just occasionally overlap.  That seems bad.  Perhaps there should just be a global counter.

Similar changes could go into testtools which apparently has a similar concept.
-- 
https://code.launchpad.net/~mbp/launchpad/uniqueid-clue/+merge/61153
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~mbp/launchpad/uniqueid-clue into lp:launchpad.

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2011-05-12 21:33:10 +0000
+++ lib/lp/testing/factory.py	2011-05-16 18:41:20 +0000
@@ -38,6 +38,7 @@
 import os
 from random import randint
 from StringIO import StringIO
+import sys
 from textwrap import dedent
 from threading import local
 from types import InstanceType
@@ -422,7 +423,10 @@
             defaults to 'generic-string'.
         """
         if prefix is None:
-            prefix = "generic-string"
+            frame = sys._getframe(1)
+            prefix = 'unique-%s-line%d-' % (
+                frame.f_code.co_filename.rsplit('/', 1)[-1],
+                frame.f_lineno)
         string = "%s%s" % (prefix, self.getUniqueInteger())
         return string.replace('_', '-').lower()
 
@@ -2560,11 +2564,11 @@
         if url is None:
             url = 'http://%s:8221/' % self.getUniqueString()
         if name is None:
-            name = self.getUniqueString()
+            name = self.getUniqueString('builder-name')
         if title is None:
-            title = self.getUniqueString()
+            title = self.getUniqueString('builder-title')
         if description is None:
-            description = self.getUniqueString()
+            description = self.getUniqueString('description')
         if owner is None:
             owner = self.makePerson()
 
@@ -2616,9 +2620,10 @@
             distroseries = self.makeSourcePackageRecipeDistroseries()
 
         if name is None:
-            name = self.getUniqueString().decode('utf8')
+            name = self.getUniqueString('spr-name').decode('utf8')
         if description is None:
-            description = self.getUniqueString().decode('utf8')
+            description = self.getUniqueString(
+                'spr-description').decode('utf8')
         if daily_build_archive is None:
             daily_build_archive = self.makeArchive(
                 distribution=distroseries.distribution, owner=owner)