Nir Soffer has posted comments on this change. Change subject: Move host stats dict creation from API.py to host module ......................................................................
Patch Set 6: (1 comment) https://gerrit.ovirt.org/#/c/56874/6/lib/vdsm/host.py File lib/vdsm/host.py: Line 74: Line 75: return host_UUID Line 76: Line 77: Line 78: def getStats(irs, cif, haClient): > Moving the code out of API.py is a good change. Having a package can work, but do we have enough code for a package? Code that needs host uuid is doing now: import host ... host.uuid This is great! If we build a package, you will have: import host.info Or from vdsm.host import info as hostinfo ... hostinfo.uuid This sucks. Same for host.get_stats, host.send_stats, etc. We can use __init__ to get the names into the host namespace, but this means we should really have a host module, and not a package. A package is needed if there is too much code or mess in a module. Do we have such problem here? Line 79: """ Line 80: Retreive host internal statistics Line 81: """ Line 82: hooks.before_get_stats() -- To view, visit https://gerrit.ovirt.org/56874 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I626a673c429d486ac78b309c2f31cb58af8852b2 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Irit Goihman <igoih...@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: 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/mailman/listinfo/vdsm-patches
