Merge lp:~linaro-infrastructure/launchpad/workitems-migration-script into lp:launchpad

[Guilherme Salgado]

[r=jcsackett,stevenk][bug=940449] Add a garbo job to migrate work items from specs' whiteboards

This is the script to migrate work items from the whiteboard to the new table. We're not going to run it until we have the new UI to edit work items separately from the whiteboard, though.

[Steve Kowalik (stevenk)]

I'm going to mark this as Needs Fixing. This requires discussion, and certainly doesn't require a one-shot script -- the usual modus operandi for migrations like this is either a garbo job or an API script.

review: Needs Fixing (code)

[Guilherme Salgado (salgado)]

On 22/02/12 00:48, Steve Kowalik wrote:
> Review: Needs Fixing code
>
> I'm going to mark this as Needs Fixing. This requires discussion, and certainly doesn't require a one-shot script -- the usual modus operandi for migrations like this is either a garbo job or an API script.

Can you let me know what is it that needs discussing? The
implementation, the migration itself or something else?

Also, the idea is to run this as a garbo script; I've implemented it
using a TunableLoop for that reason. IIUC we'd just need to add it to
the list of tasks executed by garbo, which I haven't done because the UI
hasn't landed yet.

From testing it against staging data, though, it seems to parse and
migrate ~90% of the whiteboards with work items, so my gut feeling is
that we won't need to run it for long.

[Steve Kowalik (stevenk)]

The implementation does. If the idea is to run it as a garbo job, then do that -- add it the loop in lib/lp/scripts/garbo.py, and test it as part of lib/lp/scripts/tests/test_garbo.py, not this separate script.

[Guilherme Salgado (salgado)]

The separate script is just to make it easy to test this on staging. Also, we don't want to hook the new TunableLoop into garbo yet as we don't want it running on production until the UI has landed. And I've put it all in a separate file to make it easier to remove it after all whiteboards have been migrated. If you feel strong about it, I can move the code/tests together with the other garbo stuff, but can you please also review the code itself as shuffling things around shouldn't change much of the actual code?

[Steve Kowalik (stevenk)]

If you want to QA this on staging, and not have it running on production then add the loop to HourlyDatabaseGarbageCollector.experimental_tunable_loops. And your removal point is moot, since we can just rollback the revision that included this garbo job. I do feel very strongly about this, and will not approve it as-is.

* Your imports do not meet our standards. There are three groups: Standard Python modules, contributed code, then internal modules, all of which must be in alphabetical order.
* Some of your assertions look to add gratuitous vertical whitespace, why?
* I'd prefer more descriptive names rather than say 'nonono' for a IPerson that doesn't exist.
* + words = text.replace('(', ' ').replace(')', ' ').replace('[', ' ').replace(']', ' ').replace('<wbr></wbr>', '').split() Really?! Surely this could be done better without having to chain six function calls together?
* SpecificationWorkitemMigratorProcess is also not needed.
* The script is not needed due to the above comment.

I was holding off on reviewing it until it moved into the garbo infrastructure, since it may change the code a fair bit.

[Steve Kowalik (stevenk)]

Another suggestion is to use a feature flag to guard this code. Then the garbo job will run on both production and staging, but will not do any work until the feature flag is turned on.

[Guilherme Salgado (salgado)]

On 28/02/12 19:25, Steve Kowalik wrote:
> If you want to QA this on staging, and not have it running on
> production then add the loop to
> HourlyDatabaseGarbageCollector.experimental_tunable_loops. And your
> removal point is moot, since we can just rollback the revision that
> included this garbo job. I do feel very strongly about this, and will
> not approve it as-is.
>
> * Your imports do not meet our standards. There are three groups:
> Standard Python modules, contributed code, then internal modules, all

Should be fixed now

> of which must be in alphabetical order. * Some of your assertions
> look to add gratuitous vertical whitespace, why? * I'd prefer more

Do you mean things like:

        self.assertThat(work_items[0], MatchesStructure.byEquality(
            assignee=None, title="A single work item",
            status=SpecificationWorkItemStatus.TODO,
            milestone=None,
            specification=spec))

I've rearranged the arguments on those so that they fit in two lines.

If you mean something else, I'd appreciate if you could be more specific.

> descriptive names rather than say 'nonono' for a IPerson that doesn't

Changed

> exist. * + words = text.replace('(', ' ').replace(')', '
> ').replace('[', ' ').replace(']', ' ').replace('<wbr></wbr>',
> '').split() Really?! Surely this could be done better without having
> to chain six function calls together? *

It certainly could, but this code is known to correctly parse all
variants of work-item headers (it was taken from
launchpad-work-items-tracker) and given that, as I said, we'll throw
this away once the migration is done, I'd rather not rewrite it to avoid
the regressions I might introduce when doing so. I know I could write
tests for all variants to ensure no regressions are introduced and I'd
be happy to do so if we were planning to keep this code around for some
reason, but that's not the case here.

> SpecificationWorkitemMigratorProcess is also not needed. * The script
> is not needed due to the above comment.

Right, removed.

>
> I was holding off on reviewing it until it moved into the garbo
> infrastructure, since it may change the code a fair bit.

It didn't change much; I only had to move stuff around and write a test
using the test infrastructure for garbo, which is probably nicer than
running the script as a subprocess.

[Steve Kowalik (stevenk)]

So I think I'm going to change my mind, sorry. I am extremly happy with this as it stands, but you will have an easier time QAing and deploying this if you change the garbo job to only run if a certain feature flag is set. I'd suggest lp.scripts.workitems_migrator or something like that.

[Guilherme Salgado (salgado)]

Ok, I've added the feature flag and guarded the new garbo job with it.

[j.c.sackett (jcsackett)]

The feature flag stuff looks good to me. I'll be speaking with Steve in a few hours and if he's cool with everything we can send it off to land. I don't want to step on his toes and he clearly has a fuller understanding of how this fits into LP.

review: Approve

[Steve Kowalik (stevenk)]

Thank you for bearing with me through my review, I am happy for this to land.

review: Approve (code)

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2012-03-01 05:12:30 +0000
+++ database/schema/security.cfg	2012-03-01 16:02:21 +0000
@@ -2160,11 +2160,13 @@
 public.codeimportevent                  = SELECT, DELETE
 public.codeimporteventdata              = SELECT, DELETE
 public.codeimportresult                 = SELECT, DELETE
+public.distribution                     = SELECT
 public.emailaddress                     = SELECT, UPDATE, DELETE
 public.hwsubmission                     = SELECT, UPDATE
 public.job                              = SELECT, INSERT, DELETE
 public.logintoken                       = SELECT, DELETE
 public.mailinglistsubscription          = SELECT, DELETE
+public.milestone                        = SELECT
 public.milestonetag                     = SELECT
 public.oauthnonce                       = SELECT, DELETE
 public.openidconsumerassociation        = SELECT, DELETE
@@ -2172,11 +2174,14 @@
 public.person                           = SELECT, DELETE
 public.potranslation                    = SELECT, DELETE
 public.potmsgset                        = SELECT, DELETE
+public.product                          = SELECT
 public.revisionauthor                   = SELECT, UPDATE
 public.revisioncache                    = SELECT, DELETE
 public.sourcepackagename                = SELECT
 public.sourcepackagerelease             = SELECT
 public.sourcepackagepublishinghistory   = SELECT, UPDATE
+public.specification                    = SELECT, UPDATE
+public.specificationworkitem            = SELECT, INSERT
 public.suggestivepotemplate             = INSERT, DELETE
 public.teamparticipation                = SELECT, DELETE
 public.translationmessage               = SELECT, DELETE

=== added file 'lib/lp/blueprints/tests/test_workitem_migration.py'
--- lib/lp/blueprints/tests/test_workitem_migration.py	1970-01-01 00:00:00 +0000
+++ lib/lp/blueprints/tests/test_workitem_migration.py	2012-03-01 16:02:21 +0000
@@ -0,0 +1,250 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+from textwrap import dedent
+
+from testtools.matchers import (
+    MatchesStructure,
+    )
+
+from lp.blueprints.enums import SpecificationWorkItemStatus
+from lp.blueprints.workitemmigration import (
+    extractWorkItemsFromWhiteboard,
+    WorkitemParser,
+    WorkItemParseError,
+    )
+from lp.testing import (
+    TestCase,
+    TestCaseWithFactory,
+    )
+from lp.testing.layers import DatabaseFunctionalLayer
+
+
+class FakeSpecification(object):
+    assignee = None
+
+
+class TestWorkItemParser(TestCase):
+
+    def test_parse_line_basic(self):
+        parser = WorkitemParser(FakeSpecification())
+        assignee, description, status = parser.parse_blueprint_workitem(
+            "A single work item: TODO")
+        self.assertEqual(
+            [None, "A single work item", SpecificationWorkItemStatus.TODO],
+            [assignee, description, status])
+
+    def test_parse_line_with_assignee(self):
+        parser = WorkitemParser(FakeSpecification())
+        assignee, description, status = parser.parse_blueprint_workitem(
+            "[salgado] A single work item: TODO")
+        self.assertEqual(
+            ["salgado", "A single work item",
+             SpecificationWorkItemStatus.TODO],
+            [assignee, description, status])
+
+    def test_parse_line_with_missing_closing_bracket_for_assignee(self):
+        parser = WorkitemParser(FakeSpecification())
+        self.assertRaises(
+            WorkItemParseError, parser.parse_blueprint_workitem,
+            "[salgado A single work item: TODO")
+
+    def test_parse_line_without_status(self):
+        parser = WorkitemParser(FakeSpecification())
+        assignee, description, status = parser.parse_blueprint_workitem(
+            "A single work item")
+        self.assertEqual(
+            [None, "A single work item", SpecificationWorkItemStatus.TODO],
+            [assignee, description, status])
+
+    def test_parse_line_with_invalid_status(self):
+        parser = WorkitemParser(FakeSpecification())
+        self.assertRaises(
+            WorkItemParseError, parser.parse_blueprint_workitem,
+            "A single work item: FOO")
+
+    def test_parse_line_without_description(self):
+        parser = WorkitemParser(FakeSpecification())
+        self.assertRaises(
+            WorkItemParseError, parser.parse_blueprint_workitem,
+            " : TODO")
+
+    def test_parse_line_with_completed_status(self):
+        parser = WorkitemParser(FakeSpecification())
+        assignee, description, status = parser.parse_blueprint_workitem(
+            "A single work item: Completed")
+        self.assertEqual(
+            [None, "A single work item", SpecificationWorkItemStatus.DONE],
+            [assignee, description, status])
+
+    def test_parse_line_with_inprogress_status(self):
+        parser = WorkitemParser(FakeSpecification())
+        assignee, description, status = parser.parse_blueprint_workitem(
+            "A single work item: INPROGRESS")
+        self.assertEqual(
+            [None, "A single work item",
+             SpecificationWorkItemStatus.INPROGRESS],
+            [assignee, description, status])
+
+    def test_parse_line_with_postpone_status(self):
+        parser = WorkitemParser(FakeSpecification())
+        assignee, description, status = parser.parse_blueprint_workitem(
+            "A single work item: POSTPONE")
+        self.assertEqual(
+            [None, "A single work item",
+             SpecificationWorkItemStatus.POSTPONED],
+            [assignee, description, status])
+
+    def test_parse_line_with_drop_status(self):
+        parser = WorkitemParser(FakeSpecification())
+        assignee, description, status = parser.parse_blueprint_workitem(
+            "A single work item: DROP")
+        self.assertEqual(
+            [None, "A single work item",
+             SpecificationWorkItemStatus.POSTPONED],
+            [assignee, description, status])
+
+    def test_parse_line_with_dropped_status(self):
+        parser = WorkitemParser(FakeSpecification())
+        assignee, description, status = parser.parse_blueprint_workitem(
+            "A single work item: DROPPED")
+        self.assertEqual(
+            [None, "A single work item",
+             SpecificationWorkItemStatus.POSTPONED],
+            [assignee, description, status])
+
+    def test_parse_empty_line(self):
+        parser = WorkitemParser(FakeSpecification())
+        self.assertRaises(
+            AssertionError, parser.parse_blueprint_workitem, "")
+
+
+class TestSpecificationWorkItemExtractionFromWhiteboard(TestCaseWithFactory):
+    layer = DatabaseFunctionalLayer
+
+    def test_None_whiteboard(self):
+        spec = self.factory.makeSpecification(whiteboard=None)
+        work_items = extractWorkItemsFromWhiteboard(spec)
+        self.assertEqual([], work_items)
+
+    def test_empty_whiteboard(self):
+        spec = self.factory.makeSpecification(whiteboard='')
+        work_items = extractWorkItemsFromWhiteboard(spec)
+        self.assertEqual([], work_items)
+
+    def test_single_work_item(self):
+        whiteboard = dedent("""
+            Work items:
+            A single work item: TODO
+            """)
+        spec = self.factory.makeSpecification(whiteboard=whiteboard)
+        work_items = extractWorkItemsFromWhiteboard(spec)
+        self.assertEqual(1, len(work_items))
+        self.assertThat(work_items[0], MatchesStructure.byEquality(
+            assignee=None, title="A single work item", milestone=None,
+            status=SpecificationWorkItemStatus.TODO, specification=spec))
+
+    def test_multiple_work_items(self):
+        whiteboard = dedent("""
+            Work items:
+            A single work item: TODO
+            Another work item: DONE
+            """)
+        spec = self.factory.makeSpecification(whiteboard=whiteboard)
+        work_items = extractWorkItemsFromWhiteboard(spec)
+        self.assertEqual(2, len(work_items))
+        self.assertThat(work_items[0], MatchesStructure.byEquality(
+            assignee=None, title="A single work item", milestone=None,
+            status=SpecificationWorkItemStatus.TODO, specification=spec))
+        self.assertThat(work_items[1], MatchesStructure.byEquality(
+            assignee=None, title="Another work item", milestone=None,
+            status=SpecificationWorkItemStatus.DONE, specification=spec))
+
+    def test_work_item_with_assignee(self):
+        person = self.factory.makePerson()
+        whiteboard = dedent("""
+            Work items:
+            [%s] A single work item: TODO
+            """ % person.name)
+        spec = self.factory.makeSpecification(whiteboard=whiteboard)
+        work_items = extractWorkItemsFromWhiteboard(spec)
+        self.assertEqual(1, len(work_items))
+        self.assertThat(work_items[0], MatchesStructure.byEquality(
+            assignee=person, title="A single work item", milestone=None,
+            status=SpecificationWorkItemStatus.TODO, specification=spec))
+
+    def test_work_item_with_nonexistent_assignee(self):
+        whiteboard = dedent("""
+            Work items:
+            [nonexistentperson] A single work item: TODO""")
+        spec = self.factory.makeSpecification(whiteboard=whiteboard)
+        self.assertRaises(ValueError, extractWorkItemsFromWhiteboard, spec)
+
+    def test_work_item_with_milestone(self):
+        milestone = self.factory.makeMilestone()
+        whiteboard = dedent("""
+            Work items for %s:
+            A single work item: TODO
+            """ % milestone.name)
+        spec = self.factory.makeSpecification(
+            whiteboard=whiteboard, product=milestone.product)
+        work_items = extractWorkItemsFromWhiteboard(spec)
+        self.assertEqual(1, len(work_items))
+        self.assertThat(work_items[0], MatchesStructure.byEquality(
+            assignee=None, title="A single work item", milestone=milestone,
+            status=SpecificationWorkItemStatus.TODO, specification=spec))
+
+    def test_work_item_with_unknown_milestone(self):
+        whiteboard = dedent("""
+            Work items for foo:
+            A single work item: TODO
+            """)
+        spec = self.factory.makeSpecification(whiteboard=whiteboard)
+        self.assertRaises(
+            WorkItemParseError, extractWorkItemsFromWhiteboard, spec)
+
+    def test_blank_line_signals_end_of_work_item_block(self):
+        whiteboard = dedent("""
+            Work items:
+            A single work item: TODO
+
+            Some random notes about this BP.
+              * This is what was discussed during UDS
+              * Oh, yeah, we need to do that too
+            """)
+        spec = self.factory.makeSpecification(whiteboard=whiteboard)
+        work_items = extractWorkItemsFromWhiteboard(spec)
+        self.assertEqual(1, len(work_items))
+        self.assertThat(work_items[0], MatchesStructure.byEquality(
+            assignee=None, title="A single work item",
+            status=SpecificationWorkItemStatus.TODO, specification=spec))
+
+    def test_whiteboard_with_all_possible_sections(self):
+        whiteboard = dedent("""
+            Work items:
+            A single work item: TODO
+
+            Meta:
+            Headline: Foo bar
+            Acceptance: Baz foo
+
+            Complexity:
+            [user1] milestone1: 10
+            """)
+        spec = self.factory.makeSpecification(whiteboard=whiteboard)
+        work_items = extractWorkItemsFromWhiteboard(spec)
+        self.assertEqual(1, len(work_items))
+        self.assertThat(work_items[0], MatchesStructure.byEquality(
+            assignee=None, title="A single work item", milestone=None,
+            status=SpecificationWorkItemStatus.TODO, specification=spec))
+
+        # Now assert that the work items were removed from the whiteboard.
+        self.assertEqual(dedent("""
+            Meta:
+            Headline: Foo bar
+            Acceptance: Baz foo
+
+            Complexity:
+            [user1] milestone1: 10""").strip(), spec.whiteboard.strip())

=== added file 'lib/lp/blueprints/workitemmigration.py'
--- lib/lp/blueprints/workitemmigration.py	1970-01-01 00:00:00 +0000
+++ lib/lp/blueprints/workitemmigration.py	2012-03-01 16:02:21 +0000
@@ -0,0 +1,161 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Helper functions for the migration of work items from whiteboards to the
+SpecificationWorkItem table.
+
+This will be removed once the migration is done.
+"""
+
+__metaclass__ = type
+__all__ = [
+    'extractWorkItemsFromWhiteboard',
+    ]
+
+import re
+
+from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
+
+from lp.blueprints.enums import SpecificationWorkItemStatus
+
+from lp.registry.interfaces.person import IPersonSet
+
+
+class WorkItemParseError(Exception):
+    """An error when parsing a work item line from a blueprint's whiteboard."""
+
+
+class WorkitemParser(object):
+    """A parser to extract work items from Blueprint whiteboards."""
+
+    def __init__(self, blueprint):
+        self.blueprint = blueprint
+
+    def _normalize_status(self, status, desc):
+        status = status.strip().lower()
+        if not status:
+            status = SpecificationWorkItemStatus.TODO
+        elif status == u'completed':
+            status = SpecificationWorkItemStatus.DONE
+        elif status in (u'postpone', u'dropped', u'drop'):
+            status = SpecificationWorkItemStatus.POSTPONED
+        else:
+            valid_statuses = SpecificationWorkItemStatus.items
+            if status not in [item.name.lower() for item in valid_statuses]:
+                raise WorkItemParseError('Unknown status: %s' % status)
+            return valid_statuses[status.upper()]
+        return status
+
+    def _parse_line(self, line):
+        try:
+            desc, status = line.rsplit(':', 1)
+        except ValueError:
+            desc = line
+            status = ""
+        assignee_name = None
+        if desc.startswith('['):
+            if ']' in desc:
+                off = desc.index(']')
+                assignee_name = desc[1:off]
+                desc = desc[off + 1:].strip()
+            else:
+                raise WorkItemParseError('Missing closing "]" for assignee')
+        return assignee_name, desc, status
+
+    def parse_blueprint_workitem(self, line):
+        line = line.strip()
+        assert line, "Please don't give us an empty line"
+        assignee_name, desc, status = self._parse_line(line)
+        if not desc:
+            raise WorkItemParseError(
+                'No work item description found on "%s"' % line)
+        status = self._normalize_status(status, desc)
+        return assignee_name, desc, status
+
+
+def milestone_extract(text, valid_milestones):
+    words = text.replace('(', ' ').replace(')', ' ').replace(
+        '[', ' ').replace(']', ' ').replace('<wbr></wbr>', '').split()
+    for milestone in valid_milestones:
+        for word in words:
+            if word == milestone.name:
+                return milestone
+    raise WorkItemParseError("No valid milestones found in %s" % words)
+
+
+def extractWorkItemsFromWhiteboard(spec):
+    work_items = []
+    if not spec.whiteboard:
+        return work_items
+    work_items_re = re.compile('^work items(.*)\s*:\s*$', re.I)
+    meta_re = re.compile('^Meta.*?:$', re.I)
+    complexity_re = re.compile('^Complexity.*?:$', re.I)
+    in_wi_block = False
+    new_whiteboard = []
+
+    target_milestones = list(spec.target.milestones)
+    wi_lines = []
+    # Iterate over all lines in the whiteboard and whenever we find a line
+    # matching work_items_re we 'continue' and store the following lines
+    # until we reach the end of the whiteboard or a line matching meta_re or
+    # complexity_re.
+    for line in spec.whiteboard.splitlines():
+        new_whiteboard.append(line)
+        wi_match = work_items_re.search(line)
+        if wi_match:
+            in_wi_block = True
+            milestone = None
+            milestone_part = wi_match.group(1).strip()
+            if milestone_part:
+                milestone = milestone_extract(
+                    milestone_part, target_milestones)
+            new_whiteboard.pop()
+            continue
+        if meta_re.search(line):
+            milestone = None
+            in_wi_block = False
+            continue
+        if complexity_re.search(line):
+            milestone = None
+            in_wi_block = False
+            continue
+
+        if not in_wi_block:
+            # We only care about work-item lines.
+            continue
+
+        if line.strip() == '':
+            # An empty line signals the end of the work-item block:
+            # https://wiki.ubuntu.com/WorkItemsHowto.
+            in_wi_block = False
+            milestone = None
+            continue
+
+        # This is a work-item line, which we don't want in the new
+        # whiteboard because we're migrating them into the
+        # SpecificationWorkItem table.
+        new_whiteboard.pop()
+
+        wi_lines.append((line, milestone))
+
+    # Now parse the work item lines and store them in SpecificationWorkItem.
+    parser = WorkitemParser(spec)
+    sequence = 0
+    for line, milestone in wi_lines:
+        assignee_name, title, status = parser.parse_blueprint_workitem(line)
+        if assignee_name is not None:
+            assignee_name = assignee_name.strip()
+            assignee = getUtility(IPersonSet).getByName(assignee_name)
+            if assignee is None:
+                raise ValueError("Unknown person name: %s" % assignee_name)
+        else:
+            assignee = None
+        workitem = removeSecurityProxy(spec).newWorkItem(
+            status=status, title=title, assignee=assignee,
+            milestone=milestone, sequence=sequence)
+        work_items.append(workitem)
+        sequence += 1
+
+    removeSecurityProxy(spec).whiteboard = "\n".join(new_whiteboard)
+    return work_items

=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py	2012-02-28 04:24:19 +0000
+++ lib/lp/scripts/garbo.py	2012-03-01 16:02:21 +0000
@@ -26,7 +26,6 @@
 import iso8601
 from psycopg2 import IntegrityError
 import pytz
-from pytz import timezone
 from storm.expr import In
 from storm.locals import (
     Max,
@@ -39,6 +38,8 @@
 from zope.security.proxy import removeSecurityProxy
 
 from lp.answers.model.answercontact import AnswerContact
+from lp.blueprints.model.specification import Specification
+from lp.blueprints.workitemmigration import extractWorkItemsFromWhiteboard
 from lp.bugs.interfaces.bug import IBugSet
 from lp.bugs.model.bug import Bug
 from lp.bugs.model.bugattachment import BugAttachment
@@ -63,6 +64,7 @@
 from lp.services.database.lpstorm import IMasterStore
 from lp.services.database.sqlbase import (
     cursor,
+    quote_like,
     session_store,
     sqlvalues,
     )
@@ -988,6 +990,80 @@
         transaction.commit()
 
 
+class SpecificationWorkitemMigrator(TunableLoop):
+    """Migrate work-items from Specification.whiteboard to
+    SpecificationWorkItem.
+
+    Migrating work items from the whiteboard is an all-or-nothing thing; if we
+    encounter any errors when parsing the whiteboard of a spec, we abort the
+    transaction and leave its whiteboard unchanged.
+
+    On a test with production data, only 100 whiteboards (out of almost 2500)
+    could not be migrated. On 24 of those the assignee in at least one work
+    item is not valid, on 33 the status of a work item is not valid and on 42
+    one or more milestones are not valid.
+    """
+
+    maximum_chunk_size = 500
+    offset = 0
+
+    def __init__(self, log, abort_time=None):
+        super(SpecificationWorkitemMigrator, self).__init__(
+            log, abort_time=abort_time)
+
+        if not getFeatureFlag('garbo.workitem_migrator.enabled'):
+            self.log.info(
+                "Not migrating work items. Change the "
+                "garbo.workitem_migrator.enabled feature flag if you want "
+                "to enable this.")
+            # This will cause isDone() to return True, thus skipping the work
+            # item migration.
+            self.total = 0
+            return
+
+        # Get only the specs which contain "work items" in their whiteboard
+        # and which don't have any SpecificationWorkItems.
+        query = "whiteboard ilike '%%' || %s || '%%'" % quote_like(
+            'work items')
+        query += (" and id not in (select distinct specification from "
+                  "SpecificationWorkItem)")
+        self.specs = IMasterStore(Specification).find(Specification, query)
+        self.total = self.specs.count()
+        self.log.info(
+            "Migrating work items from the whiteboard of %d specs"
+            % self.total)
+
+    def getNextBatch(self, chunk_size):
+        end_at = self.offset + int(chunk_size)
+        return self.specs[self.offset:end_at]
+
+    def isDone(self):
+        """See `TunableLoop`."""
+        return self.offset >= self.total
+
+    def __call__(self, chunk_size):
+        """See `TunableLoop`."""
+        for spec in self.getNextBatch(chunk_size):
+            try:
+                work_items = extractWorkItemsFromWhiteboard(spec)
+            except Exception, e:
+                self.log.info(
+                    "Failed to parse whiteboard of %s: %s" % (
+                        spec, unicode(e)))
+                transaction.abort()
+                continue
+
+            if len(work_items) > 0:
+                self.log.info(
+                    "Migrated %d work items from the whiteboard of %s" % (
+                        len(work_items), spec))
+                transaction.commit()
+            else:
+                self.log.info(
+                    "No work items found on the whiteboard of %s" % spec)
+        self.offset += chunk_size
+
+
 class BaseDatabaseGarbageCollector(LaunchpadCronScript):
     """Abstract base class to run a collection of TunableLoops."""
     script_name = None  # Script name for locking and database user. Override.
@@ -1238,6 +1314,7 @@
         UnusedSessionPruner,
         DuplicateSessionPruner,
         BugHeatUpdater,
+        SpecificationWorkitemMigrator,
         ]
     experimental_tunable_loops = []
 

=== modified file 'lib/lp/scripts/tests/test_garbo.py'
--- lib/lp/scripts/tests/test_garbo.py	2012-02-21 07:09:37 +0000
+++ lib/lp/scripts/tests/test_garbo.py	2012-03-01 16:02:21 +0000
@@ -12,6 +12,7 @@
     )
 import logging
 from StringIO import StringIO
+from textwrap import dedent
 import time
 
 from pytz import UTC
@@ -29,12 +30,14 @@
 from testtools.matchers import (
     Equals,
     GreaterThan,
+    MatchesStructure,
     )
 import transaction
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
 from lp.answers.model.answercontact import AnswerContact
+from lp.blueprints.enums import SpecificationWorkItemStatus
 from lp.bugs.model.bugnotification import (
     BugNotification,
     BugNotificationRecipient,
@@ -72,6 +75,7 @@
     UTC_NOW,
     )
 from lp.services.database.lpstorm import IMasterStore
+from lp.services.features import getFeatureFlag
 from lp.services.features.model import FeatureFlag
 from lp.services.identity.interfaces.account import AccountStatus
 from lp.services.identity.interfaces.emailaddress import EmailAddressStatus
@@ -1014,6 +1018,73 @@
         self.runHourly()
         self.assertNotEqual(old_update, bug.heat_last_updated)
 
+    def test_SpecificationWorkitemMigrator_not_enabled_by_default(self):
+        self.assertFalse(getFeatureFlag('garbo.workitem_migrator.enabled'))
+        switch_dbuser('testadmin')
+        whiteboard = dedent("""
+            Work items:
+            A single work item: TODO
+            """)
+        spec = self.factory.makeSpecification(whiteboard=whiteboard)
+        transaction.commit()
+
+        self.runHourly()
+
+        self.assertEqual(whiteboard, spec.whiteboard)
+        self.assertEqual(0, spec.work_items.count())
+
+    def test_SpecificationWorkitemMigrator(self):
+        # When the migration is successful we remove all work-items from the
+        # whiteboard.
+        switch_dbuser('testadmin')
+        milestone = self.factory.makeMilestone()
+        person = self.factory.makePerson()
+        whiteboard = dedent("""
+            Work items for %s:
+            [%s] A single work item: TODO
+
+            Work items:
+            Another work item: DONE
+            """ % (milestone.name, person.name))
+        spec = self.factory.makeSpecification(
+            product=milestone.product, whiteboard=whiteboard)
+        IMasterStore(FeatureFlag).add(FeatureFlag(
+            u'default', 0, u'garbo.workitem_migrator.enabled', u'True'))
+        transaction.commit()
+
+        self.runHourly()
+
+        self.assertEqual('', spec.whiteboard.strip())
+        self.assertEqual(2, spec.work_items.count())
+        self.assertThat(spec.work_items[0], MatchesStructure.byEquality(
+            assignee=person, title="A single work item",
+            status=SpecificationWorkItemStatus.TODO,
+            milestone=milestone, specification=spec))
+        self.assertThat(spec.work_items[1], MatchesStructure.byEquality(
+            assignee=None, title="Another work item",
+            status=SpecificationWorkItemStatus.DONE,
+            milestone=None, specification=spec))
+
+    def test_SpecificationWorkitemMigrator_parse_error(self):
+        # When we fail to parse any work items in the whiteboard we leave it
+        # untouched and don't create any SpecificationWorkItem entries.
+        switch_dbuser('testadmin')
+        whiteboard = dedent("""
+            Work items:
+            A work item: TODO
+            Another work item: UNKNOWNSTATUSWILLFAILTOPARSE
+            """)
+        spec = self.factory.makeSpecification(whiteboard=whiteboard)
+        IMasterStore(FeatureFlag).add(FeatureFlag(
+            u'default', 0, u'garbo.workitem_migrator.enabled', u'True'))
+        transaction.commit()
+
+        self.runHourly()
+
+        self.assertEqual(whiteboard, spec.whiteboard)
+        self.assertEqual(0, spec.work_items.count())
+
+
 
 class TestGarboTasks(TestCaseWithFactory):
     layer = LaunchpadZopelessLayer

=== modified file 'lib/lp/services/features/flags.py'
--- lib/lp/services/features/flags.py	2012-02-21 02:24:04 +0000
+++ lib/lp/services/features/flags.py	2012-03-01 16:02:21 +0000
@@ -280,6 +280,13 @@
      '',
      '',
      ''),
+    ('garbo.workitem_migrator.enabled',
+     'boolean',
+     ('If true, garbo will try to migrate work items from the whiteboard of '
+      'specifications.'),
+     '',
+     '',
+     ''),
     ])
 
 # The set of all flag names that are documented.

=== modified file 'lib/lp/testing/layers.py'
--- lib/lp/testing/layers.py	2012-02-03 06:54:05 +0000
+++ lib/lp/testing/layers.py	2012-03-01 16:02:21 +0000
@@ -269,7 +269,7 @@
 class BaseLayer:
     """Base layer.
 
-    All out layers should subclass Base, as this is where we will put
+    All our layers should subclass Base, as this is where we will put
     test isolation checks to ensure that tests to not leave global
     resources in a mess.