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