mooli tayer has posted comments on this change. Change subject: configfile.py utility for common config file editing operations. ......................................................................
Patch Set 6: (4 comments) http://gerrit.ovirt.org/#/c/27129/6/lib/vdsm/tool/configfile.py File lib/vdsm/tool/configfile.py: Line 96: continue Line 97: if not self.remove or self.rmstate != WITHIN: Line 98: m = confpat.match(line.rstrip()) Line 99: if m: Line 100: oldentries.add(m.group(1)) > please use group names Done Line 101: if self.prefix: Line 102: line = self.prefix + line Line 103: oldlines.append(line) Line 104: return oldlines, oldentries Line 132: self._writeSection(f) Line 133: f.writelines(oldlines) Line 134: if self.entries: Line 135: self._writeEntries(f, oldentries) Line 136: os.rename(tname, self.filename) > you need to have finally to remove the tname if something goes wrong Done Line 137: Line 138: if self.oldmod != os.stat(self.filename).st_mode: Line 139: os.chmod(self.filename, self.oldmod) Line 140: Line 142: try: Line 143: selinux.restorecon(self.filename) Line 144: except OSError: Line 145: pass # No default label for file Line 146: > yes, this is ugly... I do not like the selinux package... but I understand Other places in vdsm do the same plus this works for tests as well. Line 147: @context Line 148: def addEntry(self, key, val): Line 149: """ Line 150: add key=value unless key is already in the file. Line 203: return self.wrapped.getint('root', option) Line 204: Line 205: def read(self, path): Line 206: wrap = '[root]\n' + open(path, 'r').read() Line 207: wrap = StringIO.StringIO(wrap) > do not change time of variable, do not leak file descriptors. Done. As for the using io.StringIo it is a bit of a problem since the backported version does not support bytes (Python 2 strings) so I would have to use unicode with it: with open(path, 'r') as f: return self.wrapped.readfp( io.StringIO(unicode('[root]\n' + f.read())) ) which is not needed in python3. -- To view, visit http://gerrit.ovirt.org/27129 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If8c7ae2b562650e403fc39024f3531d44b2b9c4f Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: mooli tayer <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
