2012/10/23 Iustin Pop <[email protected]>: > On Tue, Oct 23, 2012 at 03:14:52PM +0200, Michael Hanselmann wrote: >> 2012/10/23 Iustin Pop <[email protected]>: >> > On Mon, Oct 22, 2012 at 06:28:34PM +0200, Michael Hanselmann wrote: >> >> --- a/lib/ssh.py >> >> +++ b/lib/ssh.py >> >> def GetUserFiles(user, mkdir=False, kind=constants.SSHK_DSA, >> >> - _homedir_fn=utils.GetHomeDir): >> >> + _homedir_fn=None): > […] >> > What is this change? >> >> For unittests of prepare-node-join, where I have to mock the home >> directory function. > > Yes, but even before, this was passed as a (default) argument, so you > could override it. I'm just asking whether there is a deeper reason that > is not obvious to me.
Ah yes, there is. prepare_node_join.UpdateSshRoot has to forward “_homedir_fn”, so it easier to have a default of “None” at both UpdateSshRoot and ssh.GetUserFiles. The later then inserts the actual implementation if the keyword argument is None. Without this change, both functions would have to define the actual implementation as a default. >> I'd agree, but one raises an exception while the other doesn't. I'd >> say the overall code would be bigger if I abstract it. Do you >> disagree? > > No, hence why I just commented about it, but didn't ask for changing the > code. If 'raise' would be usable in lambdas, it could work, but… Python > is too verbose. Ack, thanks for the review! Michael
