Giuseppe Lavagetto has submitted this change and it was merged.
Change subject: Fixed files and diff leaking.
......................................................................
Fixed files and diff leaking.
I fixed a shameful bug in my code that was leaking temp files and
dropping some diffs. I also introduced some form of structured logging
as well.
Change-Id: I3d09b78365615e66add96fff33190a36b9498af4
Signed-off-by: Giuseppe Lavagetto <glavage...@wikimedia.org>
---
M compare-puppet-catalogs/puppet_compare/generator.py
M compare-puppet-catalogs/puppet_compare/parser.py
2 files changed, 28 insertions(+), 8 deletions(-)
Approvals:
Giuseppe Lavagetto: Looks good to me, approved
jenkins-bot: Verified
diff --git a/compare-puppet-catalogs/puppet_compare/generator.py
b/compare-puppet-catalogs/puppet_compare/generator.py
index 8285c03..39b1871 100644
--- a/compare-puppet-catalogs/puppet_compare/generator.py
+++ b/compare-puppet-catalogs/puppet_compare/generator.py
@@ -5,6 +5,9 @@
from collections import defaultdict
from subprocess import CalledProcessError
from jinja2 import Environment, PackageLoader
+import logging
+
+log = logging.getLogger('puppet_compare')
env = Environment(loader=PackageLoader('puppet_compare', 'templates'))
@@ -34,7 +37,7 @@
def get_nodes():
- print "Walking dir %s" % app.config.get('NODE_DIR')
+ log.info("Walking dir %s", app.config.get('NODE_DIR'))
for subdir in os.walk(app.config.get('NODE_DIR')):
nodelist = subdir[2]
for node in nodelist:
@@ -48,7 +51,7 @@
def node_compile(nodename):
for puppet_version in app.config.get('PUPPET_VERSIONS'):
- print "Compiling node %s under puppet %s" % (nodename, puppet_version)
+ log.info("Compiling node %s under puppet %s" % (nodename,
puppet_version))
# Compile
cmd = '{} {} {} {}'.format(
app.config.get('COMPILE_SCRIPT'),
@@ -59,7 +62,7 @@
ruby(cmd)
def node_diff(nodename):
- print "Building diffs for node %s" % nodename
+ log.info("Building diffs for node %s" % nodename)
diff_cmd = '{} {} {} {}'.format(
app.config.get('DIFF_SCRIPT'),
nodename,
@@ -111,6 +114,11 @@
def main(nodes=None):
+ logging.basicConfig(
+ format='%(asctime)s %(levelname)s: %(message)s',
+ level=logging.INFO,
+ datefmt='[ %m/%d/%Y %H:%M:%S ]')
+
count = 0
if nodes is not None:
gen = lambda: [n for n in nodes.split(',')]
@@ -121,7 +129,7 @@
count += 1
if not count % 10:
update_index(nodelist)
- print "Doing node {}".format(node)
+ log.info("Doing node %s", node)
# If compilation or diff fails, build a report page
# for a node with errors.
try:
@@ -130,14 +138,14 @@
p = parser.DiffParser(filename)
diff = p.run()
except CalledProcessError:
- print "Error in compilation on node %s" % node
+ log.error("Error in compilation on node %s", node)
write_node_page(node, '', is_error=True)
nodelist['ERROR'].append(node)
continue
# If compilation is successful and no diffs, go on.
if not diff:
- print "No differences for node %s" % node
+ log.info("No differences for node %s", node)
write_node_page(node, '', is_ok=True)
nodelist['OK'].append(node)
continue
@@ -150,7 +158,7 @@
write_node_page(node, text_diff)
print "###\nRUN RESULTS:"
- for (k,v) in nodelist.items():
+ for (k, v) in nodelist.items():
print "%s => %d" % (k, len(v))
update_index(nodelist)
diff --git a/compare-puppet-catalogs/puppet_compare/parser.py
b/compare-puppet-catalogs/puppet_compare/parser.py
index 1000e6d..1a3f50e 100644
--- a/compare-puppet-catalogs/puppet_compare/parser.py
+++ b/compare-puppet-catalogs/puppet_compare/parser.py
@@ -2,7 +2,9 @@
import subprocess
import shlex
from tempfile import NamedTemporaryFile
+import logging
+log = logging.getLogger('puppet_compare')
def contains(haystack, needle):
return (haystack.find(needle) >= 0)
@@ -37,7 +39,7 @@
except subprocess.CalledProcessError as e:
# TODO: logging!
diff_resources = ''
- print e.output
+ log.error('Could not create the diffs; command was %s', cmd)
self.diffs.append((diff_resources, self.diffdata))
for state, fh in self.tmpfile.items():
@@ -75,6 +77,8 @@
state = None
for line in filehandle:
+ log.debug('State is: %s' % state)
+ log.debug('Parsing line %s', line.rstrip())
if not line.rstrip() and state != self.IN_DIFF:
# skip empty lines
continue
@@ -96,7 +100,14 @@
self.end_resource(state)
state = None
else:
+ log.debug('Adding line to %s', state)
self.add_line(state, line)
+
+ if state in self.tmpfile \
+ and self.tmpfile[state] is not None:
+ self.end_resource(state)
+ # We still have a last diff to compile, meh.
+ self.generate_diff()
def run(self):
with open(self.fname, 'r') as f:
@@ -107,6 +118,7 @@
if __name__ == '__main__':
# Ugly, just to do a quick test
import sys
+ logging.basicConfig(level=logging.DEBUG)
filename = sys.argv[1]
p = DiffParser(filename)
for (resource_diff, content_diff) in p.run():
--
To view, visit https://gerrit.wikimedia.org/r/130618
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I3d09b78365615e66add96fff33190a36b9498af4
Gerrit-PatchSet: 1
Gerrit-Project: operations/software
Gerrit-Branch: master
Gerrit-Owner: Giuseppe Lavagetto <glavage...@wikimedia.org>
Gerrit-Reviewer: Giuseppe Lavagetto <glavage...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
