Giuseppe Lavagetto has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/369610 )
Change subject: Switch to html representation of diffs
......................................................................
Switch to html representation of diffs
Also, drop datadiff as its representation of parameters diffs was, to
say the least, subpar.
The styling could be better, but I feel this new output is already way
better than the old one.
Change-Id: Ic733426443a4a0cdeadf3b5f2d7c386c2f4622ef
---
M puppet_compiler/differ.py
M puppet_compiler/templates/diffs.jinja2
M puppet_compiler/templates/hostpage.future.jinja2
M puppet_compiler/templates/hostpage.jinja2
M puppet_compiler/tests/test_differ.py
M setup.py
6 files changed, 65 insertions(+), 29 deletions(-)
Approvals:
Giuseppe Lavagetto: Looks good to me, approved
jenkins-bot: Verified
diff --git a/puppet_compiler/differ.py b/puppet_compiler/differ.py
index 9f5cbb6..95a5a04 100644
--- a/puppet_compiler/differ.py
+++ b/puppet_compiler/differ.py
@@ -1,7 +1,27 @@
import difflib
import json
-import datadiff
+
+def parameters_diff(orig, other, fromfile='a', tofile='b'):
+ output = "--- {f}\n+++ {t}\n\n".format(f=fromfile, t=tofile)
+ old = set(orig.keys())
+ new = set(other.keys())
+ # Parameters only in the old definition
+ only_in_old = old - new
+ only_in_new = new - old
+ diff = [k for k in (old & new) if orig[k] != other[k]]
+ # Now calculate the string length for arrow allignment, a la puppet.
+ param_len = max(map(len, (only_in_new.union(only_in_old).union(diff))))
+ param_format = "{p:<%d} => {v}\n" % param_len
+ for parameter in only_in_old:
+ output += "- " + param_format.format(p=parameter, v=orig[parameter])
+ for parameter in only_in_new:
+ output += "+ " + param_format.format(p=parameter,
v=other[parameter])
+ for parameter in diff:
+ output += "@@\n"
+ output += "- " + param_format.format(p=parameter, v=orig[parameter])
+ output += "+ " + param_format.format(p=parameter,
v=other[parameter])
+ return output
class PuppetResource(object):
@@ -66,7 +86,7 @@
]
out['content'] = content_diff
if self.parameters != other.parameters:
- out['parameters'] = datadiff.diff(
+ out['parameters'] = parameters_diff(
self.parameters, other.parameters,
fromfile='{}.orig'.format(str(self)), tofile=str(self)
)
diff --git a/puppet_compiler/templates/diffs.jinja2
b/puppet_compiler/templates/diffs.jinja2
index 3e11d7f..1ed2b52 100644
--- a/puppet_compiler/templates/diffs.jinja2
+++ b/puppet_compiler/templates/diffs.jinja2
@@ -1,26 +1,44 @@
-{% macro bigsep() %}{{ "%-80s"|format("=" * 80) }}{% endmacro %}
-{% macro smallsep() %}{{ "%-80s"|format("-" * 80) }}{% endmacro %}
-{{ bigsep() }}
-{{ "%-70s%10s"|format(host, diffs.perc_changed) }}
-{{ bigsep() }}
-Total Resources: {{ diffs.total }}
-{% if diffs.only_in_self %}Only in old:
-{% for resource in diffs.only_in_self %} {{ resource }}
-{% endfor %}{% endif %}
+<h2>Catalog differences</h2>
+<h3>Summary</h3>
+<table>
+ <tr><th>Total Resources:</th><td>{{ diffs.total }}</td></tr>
+ <tr><th>Resources added:</th><td>{{ diffs.only_in_other|length }}</td></tr>
+ <tr><th>Resources removed:</th><td>{{ diffs.only_in_self|length }}</td></tr>
+ <tr><th>Resources modified:</th><td>{{diffs.resource_diffs|length
}}</td></tr>
+ <tr><th>Change percentage:</th><td>{{diffs.perc_changed}}</td></tr>
+</table>
-{% if diffs.only_in_other %}Only in new:
-{% for resource in diffs.only_in_other %} {{ resource }}
-{% endfor %}{% endif %}
+{% if diffs.only_in_other %}
+<h3>Resources only in the new catalog</h3>
+<ul>
+{% for resource in diffs.only_in_other %}<li>{{ resource }}</li>{% endfor %}
+</ul>
+{% endif %}
+{% if diffs.only_in_self %}
+<h3>Resources only in the old catalog</h3>
+<ul>
+{% for resource in diffs.only_in_self %}<li>{{ resource }}</li>{% endfor %}
+</ul>
+{% endif %}
-{% if diffs.resource_diffs %}Resources changed:{% for diff in
diffs.resource_diffs %}
-{{ smallsep() }}
-Resource: {{ diff.resource}}
+{% if diffs.resource_diffs %}
+<h3>Resources modified</h3>
+<ul>
+{% for diff in diffs.resource_diffs %}
+ <li>{{ diff.resource}}
+ <dl>
{% if diff.parameters %}
-Parameters differences:
-{{ diff.parameters }}
+ <dd> Parameters differences:
+ <pre>{{ diff.parameters }}</pre>
+ </dd>
{% endif %}
{% if diff.content %}
-Content differences:
-{{ diff.content }}
-{% endif %}{{smallsep()}}{% endfor %}
+ <dd> Content differences:
+ <pre>{{ diff.content|join("") }}</pre>
+ </dd>
{% endif %}
+ </dl>
+ </li>
+{% endfor %}
+{% endif %}
+</ul>
diff --git a/puppet_compiler/templates/hostpage.future.jinja2
b/puppet_compiler/templates/hostpage.future.jinja2
index d047000..3b605c2 100644
--- a/puppet_compiler/templates/hostpage.future.jinja2
+++ b/puppet_compiler/templates/hostpage.future.jinja2
@@ -27,8 +27,7 @@
{% block body %}
<h1>Compilation results for {{ host }}: <span class="{{retcode}}">{{
desc }}</span></h1>
{% if retcode == "diff" %}
- <h2>Catalog differences</h2>
- <textarea rows="20" cols="82" class="source">{% include
"diffs.jinja2" %}</textarea>
+ {% include "diffs.jinja2" %}
{% endif %}
<h2>Relevant files</h2>
<ul>
diff --git a/puppet_compiler/templates/hostpage.jinja2
b/puppet_compiler/templates/hostpage.jinja2
index 3a3de79..57cf23a 100644
--- a/puppet_compiler/templates/hostpage.jinja2
+++ b/puppet_compiler/templates/hostpage.jinja2
@@ -27,8 +27,7 @@
{% block body %}
<h1>Compilation results for {{ host }}: <span class="{{retcode}}">{{
desc }}</span></h1>
{% if retcode == "diff" %}
- <h2>Catalog differences</h2>
- <textarea rows="20" cols="82" class="source">{% include
"diffs.jinja2" %}</textarea>
+ {% include "diffs.jinja2" %}
{% endif %}
<h2>Relevant files</h2>
<ul>
diff --git a/puppet_compiler/tests/test_differ.py
b/puppet_compiler/tests/test_differ.py
index 5e1e580..1c2cf4b 100644
--- a/puppet_compiler/tests/test_differ.py
+++ b/puppet_compiler/tests/test_differ.py
@@ -58,7 +58,7 @@
self.assertNotEqual(self.r, other)
@mock.patch('difflib.unified_diff')
- @mock.patch('datadiff.diff')
+ @mock.patch('puppet_compiler.differ.parameters_diff')
def test_diff_if_present(self, datadiff_mock, difflib_mock):
other = PuppetResource(self.raw_resource, future_filter)
self.assertIsNone(self.r.diff_if_present(other))
diff --git a/setup.py b/setup.py
index 16d771a..7e45c14 100755
--- a/setup.py
+++ b/setup.py
@@ -12,11 +12,11 @@
setup(
name='puppet_compiler',
- version='0.3.0',
+ version='0.3.1',
description='Tools to compile puppet catalogs as a service',
author='Joe',
author_email='glavage...@wikimedia.org',
- install_requires=['jinja2', 'requests', 'pyyaml', 'datadiff>=2.0.0'],
+ install_requires=['jinja2', 'requests', 'pyyaml'],
test_suite='nose.collector',
tests_require=['mock<1.1.0', 'nose'],
zip_safe=True,
--
To view, visit https://gerrit.wikimedia.org/r/369610
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic733426443a4a0cdeadf3b5f2d7c386c2f4622ef
Gerrit-PatchSet: 1
Gerrit-Project: operations/software/puppet-compiler
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
