Code review comment for lp:~frankban/launchpad/lpsetup-initial

...
> > Why do you need your own check_output, rather than using subprocess'
> > check_output? Ah! Because it is not in Python 2.6? If that's the reason,
> > what would you think of trying to get it from the module, and if there's an
> > AttributeError (or ImportError, however you want to work it) then you define
> > your function? That way we get the newer version, with possible bugfixes,
> if
> > it exists; and we have a chance to clarify why we are not using the stdlib's
> > function.
...
> I can get rid of check_output; my point is: like `subprocess.check_output`,
> it's just a little wrapper around `subprocess.Popen`, that does the real stuff
> (and we will not miss future bugfixes for that object).

Keep it. I think you ought to have a comment that explains why it is valuable. That's it.

...

> > As I'm sure you've realized, we should use generate_ssh_keys with
> descriptive
> > names per the webops request for setuplxc. If these become merely
> > "root_ssh_key" and "buildbot_ssh_key" then that's not what they want. They
> > want to know "why" the key is used (e.g., "lxc_communication_key" or
> similar).
> > If there's not a good name for this key that would distinguish it from other
> > keys a user might have, we should push back; but AFAICT there should be a
> good
> > name here.
>
> I propose 'id_for_lxc'... I know: we can do better.

"launchpad_lxc_id" perhaps? Getting the word "launchpad" in there would be valuable too, I think.

Thanks again

Gary