Comment 1 for bug 655559

From #launchpad-dev,

<bigjools> jml: can you look at the prod_lp buildbot output, the test_abort that we added is failing. I'm going to disable it as I don't think there's a quick fix but you might see differently.
<jml> bigjools: at a first glance it looks like a race: a bug in the slave maybe
<bigjools> jml: we're not waiting for the build to start before trying to kill it
<wgrant> The issue is more that you're waiting too long, so the test config crashes.
<jml> in either case, bug in the slave
<jml> it's a server, it should handle people asking it to do things
<bigjools> yes
<bigjools> as I discussed with wgrant above, we need to catch that ProcessAlreadyExited and also deal with trying to abort something that's not started
<jml> yeah, that sounds about right
<wgrant> Or just fix the test config to sleep a lot.
<bigjools> no :)
<wgrant> The problem is that it gets too far. Not that it doesn't get far enough.
<wgrant> Adding sleeps won't delay the test suite.
<jml> wgrant: even so, the slave should catch the ProcessAlreadyExited exception
<wgrant> jml: Indeed. It happens a bit locally.
<wgrant> But blah twisted blah confusing.
<jml> not at all.
<jml> try: kill_process(); except ProcessAlreadyExited: pass # yay less work for me
<bigjools> I wonder if in this case it should not be raising exceptions here, the vast majority of use cases don't care if it's already exited.
<bigjools> a return value would be sufficient if you need to know
<jml> bigjools: I don't know the use case here, but lots of similar code just swallows the exception
<jml> bigjools: it's not so much "kill the process" as "ensure that it's dead"
<bigjools> jml: yes - that points to a flaw in the API IMO
<jml> bigjools: file a ticket on Twisted if you'd like
<bigjools> mebbe
<wgrant> jml: OK then, "but blah twisted blah confusing blah slave blah untested."