LGTM.

Thanks,
Jose

On Wed, Feb 05, 2014 at 10:23:45PM +0100, Santi Raffa wrote:
> On Wed, Feb 5, 2014 at 6:22 PM, Jose A. Lopes <[email protected]> wrote:
> >> +  def ToDict(self, _with_private=False):
> >>      """Convert to a dict holding only standard python types.
> >
> > Do we need to change Copy() as well?
> 
> Hard to say in abstract. I have tested Ganeti without exposing those
> vales through Copy. I have not experienced particular problems. `git
> grep Copy(` says this method is usually used for disks, NICs and
> queries. None of these should have access to private or secret
> parameters. I'd follow through with the whitelisting approach I've
> used and consider this function "guilty until proven innocent."
> 
> >> @@ -1719,12 +1739,18 @@ class Cluster(TaggableObject):
> >>      """
> >>      return self.enabled_hypervisors[0]
> >>
> >> -  def ToDict(self):
> >> +  def ToDict(self, _with_private=False):
> >>      """Custom function for cluster.
> >>
> >>      """
> >>      mydict = super(Cluster, self).ToDict()
> >>
> >
> > Super call missing _with_private ?
> > Also shouldn't 'mydict' be 'bo' to be consistent with the others?
> > Alghout, I don't really know what 'bo' means... :)
> 
> No idea! I'd rather have I name that I can understand...
> 
>  ~ ~ ~
> 
> diff --git a/lib/objects.py b/lib/objects.py
> index 9a81216..2f3c005 100644
> --- a/lib/objects.py
> +++ b/lib/objects.py
> @@ -756,7 +756,8 @@ class Disk(ConfigObject):
>      self.dynamic_params = dyn_disk_params
> 
>    # pylint: disable=W0221
> -  def ToDict(self, include_dynamic_params=False):
> +  def ToDict(self, include_dynamic_params=False,
> +             _with_private=False):
>      """Disk-specific conversion to standard python types.
> 
>      This replaces the children lists of objects with lists of
> @@ -1205,7 +1206,6 @@ class Instance(TaggableObject):
>      python types.
> 
>      """
> -
>      bo = super(Instance, self).ToDict(_with_private=_with_private)
> 
>      if _with_private:
> @@ -1743,7 +1743,7 @@ class Cluster(TaggableObject):
>      """Custom function for cluster.
> 
>      """
> -    mydict = super(Cluster, self).ToDict()
> +    mydict = super(Cluster, self).ToDict(_with_private=_with_private)
> 
>      # Explicitly save private parameters.
>      if _with_private:
> diff --git a/lib/luxi.py b/lib/luxi.py
> index 28cdd82..654a7bc 100644
> --- a/lib/luxi.py
> +++ b/lib/luxi.py
> @@ -95,7 +95,6 @@ class Client(cl.AbstractClient):
>      return self.CallMethod(REQ_PICKUP_JOB, (job,))
> 
>    def SubmitJob(self, ops):
> -
>      ops_state = map(lambda op: op.__getstate__()
>                                 if not isinstance(op, objects.ConfigObject)
>                                 else op.ToDict(_with_private=True), ops)
> 
> 
> 
> -- 
> 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

Reply via email to