launchpad-reviewers team mailing list archive

[John A Meinel <john@xxxxxxxxxxxxxxxxx>]

John A Meinel has proposed merging lp:~jameinel/launchpad/lp-service into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
  Michael Hudson-Doyle (mwhudson)
Related bugs:
  #660264 bzr+ssh on launchpad should fork, not exec
  https://bugs.launchpad.net/bugs/660264


It looks like I accidentally targetted this to lp:launchpad/db-devel in the past, when it was meant to be targetted to lp:launchpad/devel. So I'm resubmitting. It has already been landed in db-devel, so there shouldn't be a reason to not land it into devel.

This adds a --pid-file option to "bzr launchpad-forking-service". Which then causes the service to Daemonize itself, and write its pid to the given file.

This was brought out of the discussion of trying to get the service rolled out into qastaging.
see https://rt.admin.canonical.com/Ticket/Display.html?=42199

It would seem that 'start-stop-daemon' could be told to do the forking and pid-file management (--background and --make-pidfile), but it seems to state that "This is a last resort, and is only meant for programs that either make no sense forking on their own, or where it's not feasible to add the code for them to do this themselves."

The change seems reasonably localized, and hopefully reasonably tested. In general with IPC, I'm not always sure how to avoid both race conditions, hangs, and spurious failures. I chose 2.0s as a way to wait for the process to cleanup properly, but not block forever and hang the test suite.

-- 
https://code.launchpad.net/~jameinel/launchpad/lp-service/+merge/42129
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jameinel/launchpad/lp-service into lp:launchpad.