I just deployed this.

I made some small additions -- primarily, removing the LaunchpadQueryForm. I
agree that the Model is needed, although I determined the Form is not.

Also the LaunchpadTrackerForm now causes a QueryModel to be created. That way,
the customs_twist.Command.update_trackers code will detect that it should
refresh that Launchpad tracker (as you pointed out).

This is totally great. Thanks!.

*  It looks like asheesh-additions.patch it making the TrackerQueryModel optional 
from the UI, but customs_twist.Command.update_trackers is based on the last 
polled date of the TrackerQueryModel. I could directly use TrackerQueryModel and 
not LaunchpadQueryModel (if the django ORM is ok with it). Is this your suggestion?

* I split the mysite/customs/bugimporters/base.py changes in to a new commit.

* I removed self.bug_urls, but changed it to a call to process_bugs not 
handle_task_data_json. This way the LaunchpadBug creation stays in one place and 
we have the to call determine_if_finished which seems to be required to stop the 
reactor if no new bugs are found. The other process_bugs methods also take the 
same list of (url, data).

This looks quite good. Just a few nitpicks:

* You don't need a LaunchpadQueryModel at all since you don't use it

* The changes to mysite/customs/bugimporters/base.py should be in their own
commit. (Good changes, though!)

* This delta:

+        # The bug data that show up in bug_collection['entries']
+        # is equivalent to what we get back if we asked for the
+        # data on that bug explicitly.
+        for bug in bug_collection['entries']:
+            self.bug_urls.append((bug['web_link'], bug))

arguably reproduces logic available in:

+    def handle_task_data_json(self, data, lp_bug):

So why don't you just call the handle_task_data_json() method?

In asheesh-additions.patch there are some changes that, effectively, make URL
models optional. I suggest adding those to your patchset.

Also, it's easier to review if you do:

git format-patch --stdout master > entire-series-in-one-file.patch

Can you address those items? Beyond those things, this looks great, and I'm
thrilled that we can likely land this quite soon.

A further thought:

The way we're modeling things now, there is one Launchpad BugTracker per project.

This is a little odd for the following case: if someone adds a bug to their
project, but then within Launchpad the same bug ID is reassigned to some other
project, we might handle things wrong.

It's something we should just make sure to cook up a test for, and be
happy-enough with the results at some point.

Three more notes:

1. I might be wrong about when a call to self.push_urls_onto_reactor() is needed.

2. Feel free to make liberal use of the FakeGetPage() object at the top of
mysite/customs/tests.py. If you use @mock.patch() to patch out the real getPage
with that one, then that will let you download arbitrary URLs (so long as you
save a copy of them somewhere in the repo) during a run of a test.

3. I can totally see how crazy this code is now. Luckily there are some ideas
for cleaning up our use of Twisted, which would de-insane it.

I don't think this is going to be done before 0.11.11, but we've made a lot of
really good progress on it! Marking as "later," but I'm hoping we can land it
for 0.11.12.

Here's the progress of work on this so far:

* armooo has a branch on Gitorious with some work:
https://gitorious.org/~armooo/openhatch/armooos-oh-mainline/commits/launchpad ,
started at the Dec 4 OpenHatch sprint.

* I took a look at it, and have some remarks as well as patches.

In general, you don't have to call self.process_queries() with the Launchpad
BugImporter class; that should be handled by the framework.

I notice the new Launchpad importer has no subclass of "BugParser". This is a
convention; it seems it's okay to not follow it. The most idiomatic use of
BugParser can be seen in bugimporters/bugzilla.py 

I added a test and some sample data and did a little bit of cleanup in the
attached patches. They should apply cleanly on top of the current state of
armooo's gitorious branch; armooo, feel free to push them on top your branch
immediately.

If we do this, we have to write our own consumer of the Launchpad API, I think.

That's life. Let's do it. Marking as 0.11.11.

This issue is tracked in issue260.