mooli tayer has uploaded a new change for review.
Change subject: configfile: remove unneeded methods. ......................................................................
configfile: remove unneeded methods.
Change-Id: Icf08bfebc83a9af5eb3c7de48f9a51d2263766fd Signed-off-by: Mooli Tayer mtayer@redhat.com --- M lib/vdsm/tool/configurators/configfile.py M tests/toolTests.py 2 files changed, 2 insertions(+), 106 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/24/36324/1
diff --git a/lib/vdsm/tool/configurators/configfile.py b/lib/vdsm/tool/configurators/configfile.py index d8b7fdd..d1e0472 100644 --- a/lib/vdsm/tool/configurators/configfile.py +++ b/lib/vdsm/tool/configurators/configfile.py @@ -83,7 +83,6 @@ version, sectionStart='## beginning of configuration section by vdsm', sectionEnd='## end of configuration section by vdsm', - prefix='# VDSM backup', lineComment='by vdsm'): if not os.path.exists(filename): raise OSError( @@ -94,7 +93,6 @@ self._context = False self._sectionStart = sectionStart self._sectionEnd = sectionEnd - self._prefix = prefix # remove 'lineComment' at 4.0. see 'Backward compatibility' self._lineComment = lineComment self._version = version @@ -104,9 +102,6 @@ raise RuntimeError("can only enter once") self._entries = {} self._context = True - self._prefixRemove = None - self._prefixAdd = None - self._section = None self._oldmod = os.stat(self._filename).st_mode self._remove = None self._rmstate = BEFORE @@ -131,11 +126,6 @@ continue
if not self._remove or self._rmstate != WITHIN: - if self._prefixRemove: - if line.startswith(self._prefix): - line = line[len(self._prefix):] - if self._prefixAdd: - line = self._prefix + line m = confpat.match(line.rstrip()) if m: oldentries.add(m.group('key')) @@ -149,11 +139,6 @@
def _end(self): return "%s-%s\n" % (self._sectionEnd, self._version) - - def _writeSection(self, f): - f.write(self._start()) - f.write(self._section) - f.write(self._end())
def _writeEntries(self, f, oldentries): f.write(self._start()) @@ -170,8 +155,6 @@ try: oldlines, oldentries = self._getOldContent() with os.fdopen(fd, 'w', ) as f: - if self._section: - self._writeSection(f) f.writelines(oldlines) if self._entries: self._writeEntries(f, oldentries) @@ -199,32 +182,6 @@ all pairs are added in a comment wrapped section. """ self._entries[key] = val - - @context - def prependSection(self, section): - """ - add 'section' in the beginning of the file. - section is added in a comment wrapped section. - - Only one section is currently supported. - """ - self._section = section - - @context - def prefixLines(self): - """ - Add self.prefix to the beginning of each line. - No editing is done on new content added by this config file. - """ - self._prefixAdd = True - - @context - def unprefixLines(self): - """ - Remove self.prefix from each line starting with it. - No editing is done on new content added by this config file. - """ - self._prefixRemove = True
@context def removeConf(self): diff --git a/tests/toolTests.py b/tests/toolTests.py index 82a2668..89a93bf 100644 --- a/tests/toolTests.py +++ b/tests/toolTests.py @@ -452,66 +452,6 @@ "key4=val\n" "# end conf-3.4.4\n")
- def testPrefixAndPrepend(self): - self._writeConf("/var/log/libvirt/libvirtd.log {\n" - " weekly\n" - "}\n") - with ConfigFile(self.tname, - version='3.4.4', - sectionStart="# start conf", - sectionEnd="# end conf", - prefix="# comment ") as conf: - conf.prefixLines() - conf.prependSection("Some text to\n" - "add at the top\n") - with open(self.tname, 'r') as f: - self.assertEqual(f.read(), - "# start conf-3.4.4\n" - "Some text to\n" - "add at the top\n" - "# end conf-3.4.4\n" - "# comment /var/log/libvirt/libvirtd.log {\n" - "# comment weekly\n" - "# comment }\n") - - def testPrefixIdempotencey(self): - original = ( - "/var/log/libvirt/libvirtd.log {\n" - " weekly\n" - "}\n" - ) - self._writeConf(original) - with ConfigFile(self.tname, - version='3.4.4', - sectionStart="# start conf", - sectionEnd="# end conf", - prefix="# comment ") as conf: - conf.prefixLines() - with open(self.tname, 'r') as f: - self.assertEqual(f.read(), - "# comment /var/log/libvirt/libvirtd.log {\n" - "# comment weekly\n" - "# comment }\n") - with ConfigFile(self.tname, - version='3.4.4', - sectionStart="# start conf", - sectionEnd="# end conf", - prefix="# comment ") as conff: - conff.unprefixLines() - with open(self.tname, 'r') as f: - self.assertEqual(f.read(), original) - - def testRemoveEntireLinePrefix(self): - self._writeConf("# comment\n") - with ConfigFile(self.tname, - version='3.4.4', - sectionStart="# start conf", - sectionEnd="# end conf", - prefix="# comment") as conf: - conf.unprefixLines() - with open(self.tname, 'r') as f: - self.assertEqual(f.read(), "\n") - def testRemoveConfSection(self): self._writeConf("key=val\n" "remove me!(see 'Backward compatibility')# by vdsm\n" @@ -523,8 +463,7 @@ with ConfigFile(self.tname, version='3.4.4', sectionStart="# start conf", - sectionEnd="# end conf", - prefix="# comment") as conf: + sectionEnd="# end conf") as conf: conf.removeConf() with open(self.tname, 'r') as f: self.assertEqual(f.read(), "key=val\n" @@ -536,7 +475,7 @@ version='3.4.4', sectionStart="# start conf", sectionEnd="# end conf") - self.assertRaises(RuntimeError, conff.prefixLines) + self.assertRaises(RuntimeError, conff.addEntry, 'key', 'val') self.assertRaises(RuntimeError, conff.removeConf)
def testHasConf(self):
oVirt Jenkins CI Server has posted comments on this change.
Change subject: configfile: remove unneeded methods. ......................................................................
Patch Set 1:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/13636/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/14593/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/14425/ : SUCCESS
automation@ovirt.org has posted comments on this change.
Change subject: configfile: remove unneeded methods. ......................................................................
Patch Set 2:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
oVirt Jenkins CI Server has posted comments on this change.
Change subject: configfile: remove unneeded methods. ......................................................................
Patch Set 2:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/15276/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/15107/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/14319/ : FAILURE
Yaniv Bronhaim has posted comments on this change.
Change subject: configfile: remove unneeded methods. ......................................................................
Patch Set 2:
(1 comment)
http://gerrit.ovirt.org/#/c/36324/2//COMMIT_MSG Commit Message:
Line 3: AuthorDate: 2014-12-21 18:04:07 +0200 Line 4: Commit: Mooli Tayer mtayer@redhat.com Line 5: CommitDate: 2015-01-20 17:05:57 +0200 Line 6: Line 7: configfile: remove unneeded methods. please explain a bit where we used to use it and why its redundant now Line 8: Line 9: Change-Id: Icf08bfebc83a9af5eb3c7de48f9a51d2263766fd
mooli tayer has posted comments on this change.
Change subject: configfile: remove unneeded methods. ......................................................................
Patch Set 2:
(1 comment)
https://gerrit.ovirt.org/#/c/36324/2//COMMIT_MSG Commit Message:
Line 3: AuthorDate: 2014-12-21 18:04:07 +0200 Line 4: Commit: Mooli Tayer mtayer@redhat.com Line 5: CommitDate: 2015-01-20 17:05:57 +0200 Line 6: Line 7: configfile: remove unneeded methods.
please explain a bit where we used to use it and why its redundant now
done Line 8: Line 9: Change-Id: Icf08bfebc83a9af5eb3c7de48f9a51d2263766fd
automation@ovirt.org has posted comments on this change.
Change subject: configfile: remove no longer used methods. vdsm is no longer involved in libvirt log rotation since 33925 and we can remove prefixLines and prependSection. ......................................................................
Patch Set 3:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
oVirt Jenkins CI Server has posted comments on this change.
Change subject: configfile: remove no longer used methods. vdsm is no longer involved in libvirt log rotation since 33925 and we can remove prefixLines and prependSection. ......................................................................
Patch Set 3:
Build Started (1/2) -> http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/17661/
oVirt Jenkins CI Server has posted comments on this change.
Change subject: configfile: remove no longer used methods. vdsm is no longer involved in libvirt log rotation since 33925 and we can remove prefixLines and prependSection. ......................................................................
Patch Set 3:
Build Started (2/2) -> http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/17832/
oVirt Jenkins CI Server has posted comments on this change.
Change subject: configfile: remove no longer used methods. vdsm is no longer involved in libvirt log rotation since 33925 and we can remove prefixLines and prependSection. ......................................................................
Patch Set 3:
Build Successful
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/17832/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/17661/ : SUCCESS
Yaniv Bronhaim has posted comments on this change.
Change subject: configfile: remove no longer used methods. vdsm is no longer involved in libvirt log rotation since 33925 and we can remove prefixLines and prependSection. ......................................................................
Patch Set 3: Verified+1
Dan Kenigsberg has posted comments on this change.
Change subject: configfile: remove no longer used methods. vdsm is no longer involved in libvirt log rotation since 33925 and we can remove prefixLines and prependSection. ......................................................................
Patch Set 3: Code-Review-1
(1 comment)
very partial review
https://gerrit.ovirt.org/#/c/36324/3/lib/vdsm/tool/configfile.py File lib/vdsm/tool/configfile.py:
Line 127: continue Line 128: Line 129: if not self._remove or self._rmstate != WITHIN: Line 130: if self._prefixRemove: Line 131: # todo: remove once from 3.5 not supported use our legacy-related comment
REQUIRE_FOR: upgrade from ovirt-3.5 Line 132: prefix = '# VDSM backup' Line 133: if line.startswith(prefix): Line 134: line = line[len(prefix):] Line 135: m = confpat.match(line.rstrip())
Yaniv Bronhaim has posted comments on this change.
Change subject: configfile: remove no longer used methods. vdsm is no longer involved in libvirt log rotation since 33925 and we can remove prefixLines and prependSection. ......................................................................
Patch Set 3:
(1 comment)
https://gerrit.ovirt.org/#/c/36324/3/lib/vdsm/tool/configfile.py File lib/vdsm/tool/configfile.py:
Line 127: continue Line 128: Line 129: if not self._remove or self._rmstate != WITHIN: Line 130: if self._prefixRemove: Line 131: # todo: remove once from 3.5 not supported
use our legacy-related comment
I don't see anywhere such comment in vdsm
look below also - # remove this if at 4.0. see 'Backward compatibility' - this also doesn't sound like some kind of our "standard"..
Maybe you meant to something else? " # for 3.5 backward compatibility " will do it for you? Line 132: prefix = '# VDSM backup' Line 133: if line.startswith(prefix): Line 134: line = line[len(prefix):] Line 135: m = confpat.match(line.rstrip())
Dan Kenigsberg has posted comments on this change.
Change subject: configfile: remove no longer used methods. vdsm is no longer involved in libvirt log rotation since 33925 and we can remove prefixLines and prependSection. ......................................................................
Patch Set 3:
(1 comment)
https://gerrit.ovirt.org/#/c/36324/3/lib/vdsm/tool/configfile.py File lib/vdsm/tool/configfile.py:
Line 127: continue Line 128: Line 129: if not self._remove or self._rmstate != WITHIN: Line 130: if self._prefixRemove: Line 131: # todo: remove once from 3.5 not supported
I don't see anywhere such comment in vdsm
REQUIRED_FOR is added when we have ugly legacy code that we want to drop in the future. Line 132: prefix = '# VDSM backup' Line 133: if line.startswith(prefix): Line 134: line = line[len(prefix):] Line 135: m = confpat.match(line.rstrip())
vdsm-patches@lists.fedorahosted.org