[csw-devel] SF.net SVN: gar:[17675] csw/mgar/gar/v2/lib/python/build_tier_list.py

Maciej (Matchek) Bliziński maciej at opencsw.org
Mon Apr 16 20:25:12 CEST 2012


Hi William,

A quick style scan. Comments inline.

Thanks for writing this!

No dia 14 de Abril de 2012 17:35,  <wbonnet at users.sourceforge.net> escreveu:
> Revision: 17675
>          http://gar.svn.sourceforge.net/gar/?rev=17675&view=rev
> Author:   wbonnet
> Date:     2012-04-14 16:35:28 +0000 (Sat, 14 Apr 2012)
> Log Message:
> -----------
> Initial commit
>
> Added Paths:
> -----------
>    csw/mgar/gar/v2/lib/python/build_tier_list.py
>
> Added: csw/mgar/gar/v2/lib/python/build_tier_list.py
> ===================================================================
> --- csw/mgar/gar/v2/lib/python/build_tier_list.py                               (rev 0)
> +++ csw/mgar/gar/v2/lib/python/build_tier_list.py       2012-04-14 16:35:28 UTC (rev 17675)
> @@ -0,0 +1,158 @@
> +#!/opt/csw/bin/python
> +
> +import sys
> +import os
> +from optparse import OptionParser
> +
> +catalog = {};
> +
> +
> +# ---------------------------------------------------------------------------------------------------------------------
> +#
> +#
> +class CommandLineParser(object):
> +    """Command line parser. This class is a helper used to retrive options from command line
> +    """
> +
> +    def __init__(self):
> +        # Create the option parser
> +        self.parser = OptionParser()
> +
> +        # Add the different command line options to the parser
> +        self.parser.add_option("-c", "--catalog", help="Defines the catalog to parse. Default is ./catalog",
> +                                action="store", dest="catalog", type="string")
> +        self.parser.add_option("-1", "--tier1-list", help="List of tier 1 packages",
> +                                action="store", dest="tier1", type="string")
> +        self.parser.add_option("-2", "--tier2-list", help="List of tier 2 packages",
> +                                action="store", dest="tier2", type="string")
> +        self.parser.add_option("-3", "--tier3-list", help="List of tier 3 packages",
> +                                action="store", dest="tier3", type="string")
> +        self.parser.add_option("-V", "--verbose", help="Activate verbose mode", action="store_true", dest="verbose")
> +
> +    def parse(self):
> +        (self.options, self.args) = self.parser.parse_args()
> +        return self.options, self.args
> +
> +# ---------------------------------------------------------------------------------------------------------------------
> +#
> +#
> +class ConfigurationParser(object):
> +    """This class is a helper providing getter and setter on the option from command line
> +    """
> +
> +    def __init__(self, args):
> +
> +        if args.verbose != None:
> +            self.verbose = args.verbose
> +        else:
> +            self.verbose = False

You can use "default=..." option when defining the flag, and avoid
this boilerplate code.

> +        if args.catalog != None:
> +            self.catalog = args.catalog
> +        else:
> +            self.catalog = "./catalog"
> +
> +        if args.tier1 != None:
> +            self.tier1 = args.tier1
> +        else:
> +            self.tier1 = "./tier1"
> +
> +        if args.tier2 != None:
> +            self.tier2 = args.tier2
> +        else:
> +            self.tier2 = "./tier2"
> +
> +        if args.tier3 != None:
> +            self.tier3 = args.tier3
> +        else:
> +            self.tier3 = "./tier3"
> +
> +    # -----------------------------------------------------------------------------------------------------------------
> +
> +    def getCatalog(self):
> +        return self.catalog
> +
> +    def getTierInputFile(self, tier):
> +       if (tier == 1):
> +               return self.tier1

This could be replaced with a dictionary:

tiers = {
  1: self.tier1,
  ...
}
return tiers[tier] if tier in tiers else None

(or maybe something less compressed, but still data driven)

> +       if (tier == 2):
> +               return self.tier2
> +
> +       if (tier == 3):
> +               return self.tier3
> +
> +       return None
> +
> +    def getTierOutputFile(self, tier):
> +       return "%(filename)s.out" % { 'filename' : self.getTierInputFile(tier) }
> +
> +    def getVerbose(self):
> +        return self.verbose

Looks like a javaism. Just let clients use youobject.verbose. If at
any stage you want to control it, you can use @property. This is the
beauty of Python!

> +# ---------------------------------------------------------------------------------------------------------------------
> +#
> +#
> +class Package:
> +       """ Defines a package. A package has a name (CSWfoo), a list of dependencies (same syntax as
> +           catalog (CSWfoo|CSWbar) and a tier (1, 2, 3 or None if undefined)
> +       """
> +       def __init__(self, name=None, depends=None, tier=None):
> +               self.name = name
> +               if (depends != "none"):
> +                       self.depends = depends
> +               else:
> +                       self.depends = None
> +               self.tier = 3
> +
> +       def setTier(self, tier):
> +               # Access to the global variable storing the catalog
> +               global catalog
> +
> +               # Check if tier is lower or undefined, then we need to do something
> +               if self.tier > tier :
> +                       # Set the new tier value
> +                       self.tier = tier
> +                       # And iterate the list of dependencies to recursivly call the setTier method
> +                       if (self.depends != None):

The idiom is: self.depends is not None

> +                               for pkg in self.depends.split('|'):
> +                                       catalog[pkg].setTier(tier)
> +
> +def main():
> +
> +       global catalog
> +       outputFile = {}
> +
> +       # Parse command line
> +       cliParser = CommandLineParser()
> +       (opts, args) = cliParser.parse()
> +        configParser = ConfigurationParser(opts)
> +
> +       # Read catalog content
> +       for line in open(configParser.getCatalog(), 'r'):

For opening files, use the 'with' statement which takes care of
closing the files afterwards:

with open(..., 'r') as fd:
  for line in fd:
    ...

> +               pkgInfo = line.split(' ')
> +               name = pkgInfo[2]
> +               depends = pkgInfo[6]
> +               catalog[name] = Package(name,depends)
> +
> +       for tier in (1 ,2 ,3):
> +               # Create the three files for outputing tier content
> +               outputFile[tier] = open(configParser.getTierOutputFile(tier), 'w')
> +
> +               # Check if the specific tier file exist
> +               if os.path.isfile(configParser.getTierInputFile(tier)):
> +                       for line in open(configParser.getTierInputFile(tier) , 'r'):
same here
> +                               name = line.split('\n')
> +                               catalog[name[0]].setTier(tier)
> +               else:
> +                       print "File %(filename)s does not exit. Skipping this file" %  { 'filename' : configParser.getTierInputFile(tier) }
> +
> +       for pkg in catalog:
> +               outputFile[catalog[pkg].tier].write("%(name)s\n" % { 'name' : catalog[pkg].name })
> +
> +       for tier in (1 ,2 ,3):
> +               outputFile[tier].close()
> +
> +# On sort en rendant le code de retour de main
> +if __name__ == '__main__':
> +    sys.exit(main())
>
>
> Property changes on: csw/mgar/gar/v2/lib/python/build_tier_list.py
> ___________________________________________________________________
> Added: svn:executable
>   + *
>
> This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.
>
> _______________________________________________
> devel mailing list
> devel at lists.opencsw.org
> https://lists.opencsw.org/mailman/listinfo/devel


More information about the devel mailing list