[Lustre-devel] [Lustre-shine-commits] SF.net SVN: lustre-shine:[43] branches/0.9-clusterdb/lib/Shine

[Aurelien Degremont]

Quelques petites remarques pas méchantes :)



 > +(dans Commands/Tune.py)
 > +            tuning = TuningModel(filename="/etc/shine/tuning.conf")

Ne pas oublier que le fichier peut être changé dans shine.conf :)
tuning = TuningModel(filename=Globals().get_tuning_file())



> +    def __open_file(self):    
> +        """
> +        Function used to open the tuning configuration file specified in the __filename attribute
> +        """
> +        if self.__file != None:
> +            # If the fil is already open raise an exception
> +            raise Exception("File %s is already open" %(self.__filename))
> +        else:
> +            try:    
> +                # Open the file
> +                self.__file = open(self.__filename, "r")
> +            except IOError, e:
> +                    raise Exception("Failed to open the tuning configuration file : %s" %(str(e)))
> +            

- Fait attention à ne pas lever des exceptions 'Exception', fait une 
sous-classe avec un nom explicit et une petite ligne de doc.
Sinon, un appelant ne peut pas 'catcher' cette erreur là en particulier 
et laisser remonter les autres. (Ne pas faire non plus 1 class par raise 
dans le code :), certaines sont certainement du meme type :-)


Es-tu sur d'avoir besoin d'un __open_file, __close_file alors qu'ils ne 
sont appelé que dans __load_file ?
Je pense que tu peux simplifier tout ca :)


- Je me demandais, à propos de la commande Tuning, dans quelle mesure 
vous êtes obligé de coller à la syntaxe de fichiers tuning.conf de 
LustreUtil ?
Ne serait-ce pas l'occasion de la rendre plus user-friendly?



-- 
Aurelien Degremont
CEA