Yaniv Bronhaim has posted comments on this change. Change subject: Adding report_stats to virt.stats ......................................................................
Patch Set 13: (3 comments) https://gerrit.ovirt.org/#/c/59066/13//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2016-06-13 16:47:52 +0300 Line 4: Commit: Yaniv Bronhaim <ybron...@redhat.com> Line 5: CommitDate: 2016-07-21 18:19:51 +0300 Line 6: Line 7: Adding report_stats to virt.stats > I know it's annoying, but commit message is part of the patch, and this one its not annoying at all, I'll try to elaborate more Line 8: Line 9: Change-Id: Idf494c6a3087d04c12731c587b619f253dd51165 https://gerrit.ovirt.org/#/c/59066/13/lib/vdsm/virt/vmstats.py File lib/vdsm/virt/vmstats.py: PS13, Line 195: try: : for vm_uuid in vms_stats: > If any VM lacks a single key, no stats are reported. Is that expected? Why? we discussed about it in host.report_stats, and we decided that having missing key is a bug and it should not happen. if it does, its a bug and we want to fail on it and fix it - have initial value for any field. PS13, Line 261: metrics.send(report) > Is there a particular reason why this has to be in the try block? same. I don't want to send anything on fail. having missing info is a bug -- To view, visit https://gerrit.ovirt.org/59066 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idf494c6a3087d04c12731c587b619f253dd51165 Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@redhat.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@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/admin/lists/vdsm-patches@lists.fedorahosted.org
