Francesco Romani has posted comments on this change. Change subject: virt: stats: make compute_latency more robust ......................................................................
Patch Set 9: Code-Review-1 (2 comments) https://gerrit.ovirt.org/#/c/50593/9/vdsm/virt/vmstats.py File vdsm/virt/vmstats.py: Line 325: last_sample, last_index, mode): Line 326: operations = ( Line 327: last_sample['block.%d.%s.reqs' % (last_index, mode)] - Line 328: first_sample['block.%d.%s.reqs' % (first_index, mode)] Line 329: ) > I hate this style - please avoid it. this is indeed nicer, but a few lines below we'll need another iteration, for `*.times'. Then I'll need to disambiguate the identifiers, like first_reqs = ... last_reqs = ... ... first_times = ... last_times = ... and I'm not sure this improves the readability after all. Line 330: if not operations: Line 331: return 0 Line 332: elapsed_time = ( Line 333: last_sample['block.%d.%s.times' % (last_index, mode)] - Line 344: ('writeLatency', 'wr'), Line 345: ('flushLatency', 'fl')): Line 346: try: Line 347: stats[name] = str(_compute_latency(first_sample, first_index, Line 348: last_sample, last_index, mode)) > I commented (maybe on another patch) about separating lookup (with KeyError Yes, and I understood your remark (sorry about not making this clear). I'm not sure about the best way to implement your comment in this specific patch. Line 349: except KeyError: Line 350: pass Line 351: Line 352: return stats -- To view, visit https://gerrit.ovirt.org/50593 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie4670309d48bd65a3afaba8124407598b76d808d Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com> Gerrit-Reviewer: Milan Zamazal <mzama...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
