Code review comment for lp:~jameinel/launchpad/lp-service

First off, my apologies for accidentally including some db changes in this patch.

I went for a default socket location of '/var/run/launchpad_forking_service.sock' and an lp configured '/var/tmp/launchpad_forking_service.sock', since that matched a lot of the other stuff in schema-lazr.conf. I assume we can set it wherever we want in the end.

...

> It seems like this might spin if the server socket's closed and there are no dead children.

I'm not sure how you would get the server socket closed inside this process. Regardless, children shouldn't affect whether it spins or not. We just need a way to set '_should_terminate', which can be done via SIGINT or SIGTERM.

So, yes, if self._server_socket.accept() always raises an error but the should terminate event is not set, we will spin.

I suppose I could put a "self._should_terminate.set()" inside the "EBADF" check. Would that be better for you?

I do put PID into the log file, but bzr does not for each mutter() request. So when spawning I indicate that PID started, etc. But each individual message doesn't. mutter() does its own formatting, so it is a matter of changing these lines:
=== modified file 'bzrlib/trace.py'
--- bzrlib/trace.py 2010-06-28 02:41:22 +0000
+++ bzrlib/trace.py 2010-09-22 20:15:56 +0000
@@ -183,7 +183,7 @@
     else:
         out = fmt
     now = time.time()
- timestamp = '%0.3f ' % (now - _bzr_log_start_time,)
+ timestamp = '%0.3f [%d] ' % (now - _bzr_log_start_time, os.getpid())
     out = timestamp + out + '\n'
     _trace_file.write(out)
     # there's no explicit flushing; the file is typically line buffered.