cf-charmers team mailing list archive

[Cory Johns <cory.johns@xxxxxxxxxxxxx>]

Cory Johns has proposed merging lp:~johnsca/charms/trusty/cloudfoundry/charmgen-release-diff-improvements into lp:~cf-charmers/charms/trusty/cloudfoundry/trunk.

Requested reviews:
  Cloud Foundry Charmers (cf-charmers)

For more details, see:
https://code.launchpad.net/~johnsca/charms/trusty/cloudfoundry/charmgen-release-diff-improvements/+merge/243723

I was finding the diff output impossible to follow, so I made a yaml format that I think much more clearly presents the differences between releases that we need to handle.

Sample output of the new format: http://pastebin.ubuntu.com/9371409/
-- 
Your team Cloud Foundry Charmers is requested to review the proposed merge of lp:~johnsca/charms/trusty/cloudfoundry/charmgen-release-diff-improvements into lp:~cf-charmers/charms/trusty/cloudfoundry/trunk.

=== modified file 'charmgen/differ.py'
--- charmgen/differ.py	2014-07-10 02:20:07 +0000
+++ charmgen/differ.py	2014-12-04 19:25:57 +0000
@@ -4,31 +4,57 @@
 import yaml
 
 from datadiff import diff
+from getrels import find_jobs
 from getrels import parse_revs
+from getrels import get_repo
+from getrels import run_rev
 
 
 def setup():
-    parser = argparse.ArgumentParser()
+    parser = argparse.ArgumentParser(epilog='\n'.join([
+        'If the --repo-path option is given, the revisions will be diffed',
+        'directly from that.  Otherwise, output files from getrels.py',
+        '(e.g., output-v180.yaml) will be expected.',
+    ]))
     parser.add_argument('-s', '--summary', action='store_true')
+    parser.add_argument('-f', '--format',
+                        choices=['diff', 'yaml'], default='yaml')
+    parser.add_argument('-r', '--repo-path', help='path to the cf-release repo')
     parser.add_argument('revs', nargs="+")
     options = parser.parse_args()
 
+    repo = None
+    if options.repo_path:
+        options.verbose = False
+        options.directory = options.repo_path
+        options.keep_defaults = True
+        find_jobs(options)
+        repo = get_repo(options.repo_path)
+
     data = []
     revs = map(str, parse_revs(options.revs))
     for r in revs:
-        if not os.path.exists(r):
-            if not r.startswith('v'):
-                r = 'output-v%s.yaml' % r
-                if not os.path.exists(r):
-                    continue
-
-        with open(r) as fp:
-            data.append(yaml.safe_load(fp))
+        if repo:
+            result = run_rev(repo, r, options)
+            if result:
+                data.append((r, result['data']))
+            else:
+                print 'Unable to get data for %s; skipping' % r
+                continue
+        else:
+            for filename in ['output-%s%s.yaml' % (v, r) for v in ('v', '')]:
+                if os.path.exists(r):
+                    break
+            else:
+                print 'Output file for %s not found; skipping' % r
+                continue
+            with open(filename) as fp:
+                data.append((r, yaml.safe_load(fp)))
     return options, data
 
 
 def summarize(a, b):
-    print "{}\t{}\t{}\t{}".format(a['revision'][1:],
+    print "{}\t{}\t{}\t{}".format(a['revision'],
                                   len(a['interfaces']),
                                   len(a['relations']),
                                   b['revision'])
@@ -63,21 +89,57 @@
         print output
 
 
+def get_new_properties(a, b):
+    result = {}
+    for intf in b.keys():
+        for prop, value in b[intf].items():
+            if value is None and prop not in a.get(intf, {}):
+                result[intf] = sorted(result.get(intf, []) + [prop])
+    return result
+
+
+def get_relation_changes(a, b):
+    result = {}
+
+    def normalize(rel_a, rel_b):
+        result = sorted([rel_a, rel_b])
+        return tuple(result)
+
+    for rel in b.keys():
+        added = set(b[rel]) - set(a.get(rel, []))
+        removed = set(a.get(rel, [])) - set(b[rel])
+        for orel in added:
+            result.setdefault('added', set()).add(normalize(rel, orel))
+        for orel in removed:
+            result.setdefault('removed', set()).add(normalize(rel, orel))
+    if 'added' in result:
+        result['added'] = sorted(result['added'], key=lambda r: r[0])
+    if 'removed' in result:
+        result['removed'] = sorted(result['removed'], key=lambda r: r[0])
+    return result
+
+
 def main():
     options, data = setup()
-    last = len(data) - 1
-    for i, a in enumerate(data):
-        if i + 1 > last:
-            break
-        b = data[i + 1]
+    for i in range(len(data)-1):
+        rev_a, a = data[i]
+        rev_b, b = data[i + 1]
+        print '{0} {1} -> {2} {0}'.format('=' * 20, rev_a, rev_b)
         if options.summary:
             summarize(a, b)
-        else:
+        elif options.format == 'diff':
             diffkey(a, b, 'interfaces')
             diffkey(a, b, 'relations')
+        elif options.format == 'yaml':
+            print yaml.safe_dump({
+                'new properties': get_new_properties(a['interfaces'], b['interfaces']),
+            }, default_flow_style=False)
+            print yaml.safe_dump({
+                'relations': get_relation_changes(a['relations'], b['relations']),
+            })
 
     if len(data) > 2:
-        summarize(data[0], data[-1])
+        summarize(data[0][1], data[-1][1])
 
 if __name__ == '__main__':
     main()

=== modified file 'charmgen/getrels.py'
--- charmgen/getrels.py	2014-08-18 21:28:22 +0000
+++ charmgen/getrels.py	2014-12-04 19:25:57 +0000
@@ -45,11 +45,11 @@
 def get_rev(repo, rev=None):
     refs = repo.references
     if rev:
-        if isinstance(rev, int) and not str(rev).startswith('v'):
-            rev = 'v%s' % rev
-        try:
+        if 'v%s' % rev in refs:
+            return refs['v%s' % rev]
+        elif rev in refs:
             return refs[rev]
-        except IndexError:
+        else:
             return None
 
     # find the max
@@ -147,13 +147,12 @@
     for k, v in relations.items():
         relations[k] = sorted(v)
 
-    with open('output-%s.yaml' % ref.name, 'w') as fp:
-        yaml.safe_dump(
-            dict(interfaces=interfaces,
-                 relations=relations,
-                 revision=ref.name),
-            fp, default_flow_style=False)
-    return summary
+    return {
+        'summary': summary,
+        'data': {'interfaces': interfaces,
+                 'relations': relations,
+                 'revision': ref.name},
+    }
 
 
 def main():
@@ -167,7 +166,9 @@
     for rev in revs:
         result = run_rev(repo, rev, options)
         if rev is not None and result is not None:
-            summary.append([rev, result])
+            summary.append([rev, result['summary']])
+            with open('output-%s.yaml' % rev, 'w') as fp:
+                yaml.safe_dump(result['data'], fp, default_flow_style=False)
 
     yaml.safe_dump(summary, sys.stdout)