[MediaWiki-commits] [Gerrit] Fixed files and diff leaking. - change (operations/software)
Giuseppe Lavagetto has uploaded a new change for review.
https://gerrit.wikimedia.org/r/130618
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(-)
git pull ssh://gerrit.wikimedia.org:29418/operations/software
refs/changes/18/130618/1
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
[MediaWiki-commits] [Gerrit] Fixed files and diff leaking. - change (operations/software)
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)
