Code review comment for lp:~allenap/python-pgbouncer/reliable-shutdown

> > * I guess if the test dies, pgbouncer will now die as well so you don't
> need
> > a pidfile to set things right during a later run?
>
> Well, I don't know if there are any greater guarantees than
> before. Even if we/pgbouncer arranges for SIGHUP to be sent on parent
> death it will only cause pgbouncer to reload its config.
>
> I suppose I could add an atexit handler to kill child processes.

If pgbouncer is a child of the test process, it should die along with the process. Don't mess with atexit if you can avoid it. It'd be good to give this a manual try though, just in case. You can probably just insert a long sleep and issue a manual “kill -segv <pid>” to see what happens if the test process dies without any python exit handling whatsoever.

> > * Why treat the files you open as binary? Take a pidfile, for instance. I
> > don't suppose it'll ever matter, but isn't that a text file? Adding the "b"
> > to the open modes seems to confuse more than help.
>
> Just a habit. The Python docs state that using "b" improves
> portability. Not that it's needed, but it does no harm, and the
> "features" of text mode are not needed anyway.

AFAIK it only improves portability for files that aren't pure text!