LGTM. Thanks, Jose
On Thu, Feb 06, 2014 at 01:09:49PM +0100, Santi Raffa wrote: > On Thu, Feb 6, 2014 at 10:40 AM, Jose A. Lopes <[email protected]> wrote: > > Ok. Just leave it as it is. > > But I'm curious as to how it broke your code. > > Oh, I was ultimately trying to read an INI section I never wrote. > > > Unfortunately, in Ganeti we always make the keyword args explicit. > > So noooooo. :) > > :( In my modest opinion this is not a readability improvement: > > diff --git a/lib/cmdlib/cluster.py b/lib/cmdlib/cluster.py > index 09cea79..556384b 100644 > --- a/lib/cmdlib/cluster.py > +++ b/lib/cmdlib/cluster.py > @@ -1241,9 +1241,11 @@ class LUClusterSetParams(LogicalUnit): > # Remove os validity check > changed_oses = (set(self.new_osp.keys()) | > set(self.new_osp_private.keys())) > for os_name in changed_oses: > - os_params = cluster.SimpleFillOS(os_name, > - self.new_osp.get(os_name, {}), > - self.new_osp_private.get(os_name, {})) > + os_params = cluster.SimpleFillOS( > + os_name, > + self.new_osp.get(os_name, {}), > + osparams_private=self.new_osp_private.get(os_name, {}) > + ) > # check the parameter validity (remote check) > CheckOSParams(self, False, [self.cfg.GetMasterNode()], > os_name, os_params) > diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py > index 2f38a01..c644b1c 100644 > --- a/lib/cmdlib/instance.py > +++ b/lib/cmdlib/instance.py > @@ -869,7 +869,8 @@ class LUInstanceCreate(LogicalUnit): > if name in os_defs and os_defs[name] == self.op.osparams[name]: > del self.op.osparams[name] > > - os_defs_ = cluster.SimpleFillOS(self.op.os_type, {}, {}) > + os_defs_ = cluster.SimpleFillOS(self.op.os_type, {}, > + osparams_private={}) > for name in self.op.osparams_private.keys(): > if name in os_defs_ and os_defs_[name] == > self.op.osparams_private[name]: > del self.op.osparams_private[name] > @@ -983,10 +984,12 @@ class LUInstanceCreate(LogicalUnit): > if self.op.osparams_secret is None: > self.op.osparams_secret = serializer.PrivateDict() > > - self.os_full = cluster.SimpleFillOS(self.op.os_type, > - self.op.osparams, > - self.op.osparams_private, > - self.op.osparams_secret) > + self.os_full = cluster.SimpleFillOS( > + self.op.os_type, > + self.op.osparams, > + osparams_private=self.op.osparams_private, > + osparams_secret=self.op.osparams_secret > + ) > > # now that hvp/bep are in final format, let's reset to defaults, > # if told to do so > > > > > -- > Raffa Santi > Google Germany GmbH > Dienerstr. 12 > 80331 München > > > Registergericht und -nummer: Hamburg, HRB 86891 > Sitz der Gesellschaft: Hamburg > Geschäftsführer: Graham Law, Christine Elizabeth Flores -- Jose Antonio Lopes Ganeti Engineering Google Germany GmbH Dienerstr. 12, 80331, München Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Graham Law, Christine Elizabeth Flores Steuernummer: 48/725/00206 Umsatzsteueridentifikationsnummer: DE813741370
