Francesco Romani has posted comments on this change. Change subject: lib: utils: consolidate Error class in one place ......................................................................
Patch Set 1: (6 comments) https://gerrit.ovirt.org/#/c/47964/1/lib/vdsm/taskset.py File lib/vdsm/taskset.py: Line 19: # Line 20: Line 21: from __future__ import absolute_import Line 22: Line 23: from .utils import Error > Francesco, I don't think you cut corners, having an alias of utils.Error he OK, then let's have the alias as you showed, which avoids one from ... import. Line 24: from . import constants Line 25: from . import utils Line 26: Line 27: Line 32: this is the only usecase VDSM cares about - and requires. Line 33: Return a frozenset of ints, each one being a cpu indices on which the Line 34: process can run. Line 35: Example: frozenset([0, 1, 2, 3]) Line 36: Raise Error on failure. > I think it is better to keep the module name, it make it clear what is the will drop this change, since we will have aliases. Line 37: """ Line 38: command = [constants.EXT_TASKSET, '--pid', str(pid)] Line 39: Line 40: rc, out, err = utils.execCmd(command, resetCpuAffinity=False) Line 52: the target process. Line 53: <cpu_set> must be an iterable whose items are ints which represent Line 54: cpu indices, on which the process will be allowed to run; the format Line 55: is the same as what the get() function returns. Line 56: Raise Error on failure. > Same Done Line 57: """ Line 58: command = [constants.EXT_TASKSET] Line 59: if all_tasks: Line 60: command.append("--all-tasks") https://gerrit.ovirt.org/#/c/47964/1/lib/vdsm/udevadm.py File lib/vdsm/udevadm.py: Line 23 Line 24 Line 25 Line 26 Line 27 > I wonder if we should do intead: Could be good solution for a future patch, when we have a clear need. For this patch I'll stick with aliases, being the simplest solution that works. Line 19: # Line 20: Line 21: from __future__ import absolute_import Line 22: import logging Line 23: from .utils import Error > Same as taskset. Done Line 24: from . import utils Line 25: Line 26: _UDEVADM = utils.CommandPath("udevadm", "/sbin/udevadm", "/usr/sbin/udevadm") Line 27: https://gerrit.ovirt.org/#/c/47964/1/lib/vdsm/utils.py File lib/vdsm/utils.py: Line 674: Line 675: return p.returncode, out, err Line 676: Line 677: Line 678: class Error(Exception): > Lets move this into cmdutils. way better, thanks. execCmd will be moved in a future patch. Line 679: Line 680: def __init__(self, rc, out, err): Line 681: self.rc = rc Line 682: self.out = out -- To view, visit https://gerrit.ovirt.org/47964 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia490c055e6e5c637eba82ccffb9d2dfc748db8f5 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <from...@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: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches