Alexey Serbin has posted comments on this change. Change subject: KUDU-1571. Generate "deprecated" metrics in MDL ......................................................................
Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/4067/1/java/kudu-csd/check_csd_compatibility.py File java/kudu-csd/check_csd_compatibility.py: Line 34: Return a set() representing the metrics in the given MDL JSON object. nit: set() --> set ? Line 42: for entity_def in mdl['metricEntityTypeDefinitions']: What if 'metricDefinitions' key is absent in the MDL? Should it be an error or the script should with that like if the list of metric type entities is empty? Line 62: for m_type, m_entity, m_name in added_metrics: nit: consider using '_' for unused fields: for _, m_entity, m_name in added_metrics: ... Line 64: if len(removed_metrics): > nit: This isn't very pythonic. Should be `if removed_metrics:`, I think. nit: consider writing it with less code duplication, like for metrics, tag in [(added_metrics, "Added"), (removed_metrics), "Removed"]: print "%s %d metric(s):" % (tag, len(metrics)) for _, m_entity, m_name in metrics: print " %s metric %s" % (m_entity, m_name) print "Compatibility check %s" % ("FAILED" if removed_metrics else "PASSED") PS1, Line 77: May also generate a file containing JSON Probably I missed something, but I could not see how this script can do this without a slight modification :) -- To view, visit http://gerrit.cloudera.org:8080/4067 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I121203d781ab593cb94d7248d164cdad7bf26e78 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
