* Michele Tartara <[email protected]> [2014-02-13 16:20:22 +0100]: > On Thu, Feb 13, 2014 at 3:46 PM, Dimitris Aragiorgis <[email protected]> wrote: > > During backup import/export SafeConfigParser() is used to > > save/restore instance's configuration. There is a possibility if an > > export is done with a different Ganeti version, a specific value not > > to be saved during export (e.g. the NIC/Disk name) but still > > requested during import. > > > > With this patch we override the get() method of SafeConfigParser() > > and catch NoOptionError if raised and return None. Additionally we > > translate "None" values read from .ini file into None. > > > > Signed-off-by: Dimitris Aragiorgis <[email protected]> > > --- > > lib/objects.py | 15 ++++++++++++++- > > 1 file changed, 14 insertions(+), 1 deletion(-) > > > > diff --git a/lib/objects.py b/lib/objects.py > > index 358f108..6aca6fc 100644 > > --- a/lib/objects.py > > +++ b/lib/objects.py > > @@ -2159,7 +2159,8 @@ class Network(TaggableObject): > > return obj > > > > > > -class SerializableConfigParser(ConfigParser.SafeConfigParser): > > +# need to inherit object in order to use super() > > +class SerializableConfigParser(ConfigParser.SafeConfigParser, object): > > """Simple wrapper over ConfigParse that allows serialization. > > > > This class is basically ConfigParser.SafeConfigParser with two > > @@ -2181,6 +2182,18 @@ class > > SerializableConfigParser(ConfigParser.SafeConfigParser): > > cfp.readfp(buf) > > return cfp > > > > + def get(self, section, option, **kwargs): > > + value = None > > + try: > > + value = super(SerializableConfigParser, self).get(section, option, > > + **kwargs) > > + if value.lower() == constants.VALUE_NONE: > > + value = None > > + except ConfigParser.NoOptionError: > > + pass > > + > > + return value > > + > > > > class LvmPvInfo(ConfigObject): > > """Information about an LVM physical volume (PV). > > -- > > 1.7.10.4 > > > > I think we should at least log that an error happened. Silently > accepting it seems to me extremely dangerous. >
Sure. Maybe "extremely" is too much here, but ok :) > Also, I'm not confident enough about this part of the code to say that > all the values can be optional if not found. So, what about > specifically checking whether we have been looking for the disk name > and nick name and ONLY in that case returning None instead of the > exception? Well I think this can get kinda ugly. What if another option gets added or removed in the future? Are we going to add another if: then: else? > This way, we know that we are modifying only the behavior related to > the new configuration fields. > What is actually new? Name is a NIC/Disk parameter since 2.7... How about adding just the logging? Thanks, dimara > Cheers, > Michele > > -- > 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
signature.asc
Description: Digital signature
