[pykickstart rhel7-branch/master][PATCH] Return None for attributes if no method has been set (#1002371)

[David Shea]

The method proxy was returning "url" for method.method if no method had
been set, and delegating other attributes to the URL handler. Instead,
return None until one of the handlers has been selected by setting
method.method.
---
 pykickstart/commands/method.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/pykickstart/commands/method.py b/pykickstart/commands/method.py
index fb55f25..1e0cf90 100644
--- a/pykickstart/commands/method.py
+++ b/pykickstart/commands/method.py
@@ -44,11 +44,13 @@ class FC3_Method(KickstartCommand):
                 return "nfs"
             else:
                 return getattr(self.handler.nfs, name)
-        else:
+        elif self.handler.url.seen:
             if name == "method":
                 return "url"
             else:
                 return getattr(self.handler.url, name)
+        else:
+            return None

def __setattr__(self, name, value):
         if name in self.internals:

[Vratislav Podzimek]

On Thu, 2013-08-29 at 13:31 -0400, David Shea wrote:
> The method proxy was returning "url" for method.method if no method had
> been set, and delegating other attributes to the URL handler. Instead,
> return None until one of the handlers has been selected by setting
> method.method.
>
> pykickstart/commands/method.py | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/pykickstart/commands/method.py b/pykickstart/commands/method.py
> index fb55f25..1e0cf90 100644
> --- a/pykickstart/commands/method.py
> +++ b/pykickstart/commands/method.py
> @@ -44,11 +44,13 @@ class FC3_Method(KickstartCommand):
>                  return "nfs"
>              else:
>                  return getattr(self.handler.nfs, name)
>    else:
>    elif self.handler.url.seen:
>        if name == "method":
>            return "url"
>        else:
>            return getattr(self.handler.url, name)
>    else:
>        return None
>
> def __setattr__(self, name, value):
>        if name in self.internals:

This looks good to me, but I'm quite afraid some code relies on the old
(wrong) behaviour. Please try to test that change as much as possible
before pushing.

[David Shea]

On 08/30/2013 04:17 AM, Vratislav Podzimek wrote:
> On Thu, 2013-08-29 at 13:31 -0400, David Shea wrote:
> > The method proxy was returning "url" for method.method if no method had
> > been set, and delegating other attributes to the URL handler. Instead,
> > return None until one of the handlers has been selected by setting
> > method.method.
> >
> > pykickstart/commands/method.py | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/pykickstart/commands/method.py b/pykickstart/commands/method.py
> > index fb55f25..1e0cf90 100644
> > --- a/pykickstart/commands/method.py
> > +++ b/pykickstart/commands/method.py
> > @@ -44,11 +44,13 @@ class FC3_Method(KickstartCommand):
> >                   return "nfs"
> >               else:
> >                   return getattr(self.handler.nfs, name)
> >    else:
> >    elif self.handler.url.seen:
> >         if name == "method":
> >             return "url"
> >         else:
> >             return getattr(self.handler.url, name)
> >    else:
> >        return None
> >
> > def __setattr__(self, name, value):
> >     if name in self.internals:
>
> This looks good to me, but I'm quite afraid some code relies on the old
> (wrong) behaviour. Please try to test that change as much as possible
> before pushing.

The getattr proxy was added on July 1st, so we're probably ok, but I'll 
take a closer look. And I need to add tests for this case and the last 
change or two I've made to pykickstart.