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

Revision history for this message
Martin Pool (mbp) wrote :

On 7 October 2010 10:09, Michael Hudson-Doyle <email address hidden> wrote:
>> > 21266 pts/1    S+     0:00 /usr/bin/python /home/mwh/canonical/checkouts/trunk/eggs/bzr-2.2.0-py2.6-linux-x86_64.egg/EGG-INFO/scripts/bzr launchpad-forking-service --path /var/tmp/launchpad_forking_service.sock BRANCH:~mark/upstart/branch
>> >
>> > Maybe inserting a call to setproctitle in become_child would be useful,
>> > especially as we have the dependency already -- one piece of information
>> > we've lost is the user who has connected, although we can probably
>> > figure that out from logs still.
>>
>> Without the call to setproctitle, we would be losing this thing spm
>> said he liked, which is to immediately show which user and branch it
>> corresponds to.
>
> Uh?  No, that's the bit we retain.  The ps output I showed above was
> from running with the branch being reviewed.

The username is not shown. I don't know if it's a big deal; this
branch is old enough already.

>
>> To my mind it's more important to log the username, branch and pid, so
>> that you can see who it was both while the process is running and
>> afterwards.  It's also nice that jam logs the rusage.  But putting
>> them into the argv is more immediately accessible.
>
> I'm not sure if you're recommending this branch be changed before
> landing here.

what john said.

> That would be interesting.  The sftp server being a twisted app, access
> to this URL should really be nonblocking which always adds a wrinkle.
> Maybe that would be fine though, or maybe we can just cheat.

That's actually kind of a feature...

--
Martin

« Back to merge proposal