[PATCH 15/16] Add support for KS %addon section and the API+code to use it

Martin Sivak msivak at redhat.com
Thu Dec 13 16:59:35 UTC 2012


> > +    def handle_line(self, line):
> > +        """Process one kickstart line."""
> > +        self.content += line
> Does line end with the newline character?

Hmm.. good question, I have no idea :)
 

> > +        if self.addon_id and not hasattr(self.handler.addon,
> > self.addon_id):
> > +            setattr(self.handler.addon, self.addon_id,
> > AnacondaKSAddon(self.addon_id))
> Why are you using setattr here?

Because self.addon_id (the id of the addon) is a string we got from the filesystem and I need to set an attribute with the same name?
 
 
> > +    with progress_report(_("Configuring addons")):
>                               ^^^^ I guess we need a better word
>                               here.

Probably, but we will have real 3rd party addons too here, not just externally maintained components

> > topology = None
> >  
> > +
> >  class AnacondaKSScript(KSScript):

> No newline needed here and the same goes for the AnacondaKSHandler
> class.

That is actually PEP8 style.. two lines between classes. But I can remove it of course..
 
> > os.path.basename(os.path.dirname(os.path.abspath(path)))
> Are all os.path.* calls needed here? Why to get abspath when you use
> dirname and basename afterwards?

Because if any path contains "something/../spokes" you will get "something/.." out of dirname and ".." out of basename.
 
> > +            if not os.path.isdir(path):
> > +                continue
> > +
> > +            classes = collect(module_name, path, lambda cls:
> > issubclass(cls, self.AddonClassType))
> > +            print classes
> I guess we don't need this print.
> 
> > +            if classes:
> > +                addons[addon_id] = classes[0](name = addon_id)
> Could there be multiple classes? And if yes, how it is ensured that
> the
> right one is the first in the list?

Only one matching class per plugin was the plan. But this is still open to changes. I need to see real use cases from other teams first (ID handling would have to change if we allow more classes).



More information about the anaconda-patches mailing list