2010/2/23 Rene Nussbaumer <[email protected]>:
> On Mon, Feb 22, 2010 at 4:21 PM, Michael Hanselmann <[email protected]> wrote:
>> 2010/2/22 René Nussbaumer <[email protected]>:
>>> +class Merger(object):
>>> +  ROLLBACK_STEPS = (
>>> +      "Remove our key from authorized_keys on nodes: %(nodes)s",
>>> +      "Start all instances again on the merging clusters: %(clusters)s",
>>> +      "Restore %s from another master candidate" % 
>>> constants.CLUSTER_CONF_FILE)
>>
>> Is this not a list?
>
> Does it matter if it's a list or a tuple? Though, tuples are read-only
> and as the list is not going to be changed I like it this way.

Yes, they're two different things. A tuple is more like a data
structure on its own, whereas a list is a container. Python 2.6 and
above even have a namedtuple() function to create tuples with named
fields. We don't use tuples as “read-only” lists in Ganeti.

>> Don't pass non-keyword arguments as arguments (command is not a
>> keyword argument on SshRunner.BuildCmd).
>
> What you probably mean is supply them as positional argument, because
> every argument is a keyword argument in that sense and can be used as
> such. While I see this reduces code size (well, just minimally), is
> there any other reason to go for it?

Style and maintainability. For example, if one renames a positional
argument (for whatever reasons), not all callers have to be checked.

I'm going to send more comments directly your patch.

Regards,
Michael

Reply via email to