Milan Zamazal has posted comments on this change.

Change subject: periodic: explicitely track domain availability
......................................................................


Patch Set 9:

(3 comments)

https://gerrit.ovirt.org/#/c/47246/9/vdsm/virt/periodic.py
File vdsm/virt/periodic.py:

Line 211:     """
Line 212: 
Line 213:     _log = logging.getLogger("virt.periodic.VmDispatcher")
Line 214: 
Line 215:     def __init__(self, get_vms, executor, create, timeout, ttl):
Please document ttl argument.
Line 216:         """
Line 217:         get_vms: callable which will return a dict which maps
Line 218:                  vm_ids to vm_instances
Line 219:         executor: executor.Executor instance


Line 217:         get_vms: callable which will return a dict which maps
Line 218:                  vm_ids to vm_instances
Line 219:         executor: executor.Executor instance
Line 220:         create: callable to obtain the real callable to
Line 221:                 dispatch, with its timeout
I wouldn't mind if `timeout' was properly documented as part of this change as 
well.  (It's not clear from the docstring itself what `timeout' refers to here.)
Line 222:         """
Line 223:         self._get_vms = get_vms
Line 224:         self._executor = executor
Line 225:         self._create = create


Line 232: 
Line 233:         for vm_id, vm_obj in vms.iteritems():
Line 234:             try:
Line 235:                 to_skip = self._skip_doms.get(vm_id, False)
Line 236:                 if to_skip:
Sorry about my previously misplaced note regarding `skipped'. What I actually 
meant was: Originally, if we skipped on `not op.runnable' and op was required, 
vm_id was added to `skipped'. It's not so now under similar circumstances. Is 
it intentional?
Line 237:                     continue
Line 238: 
Line 239:                 op = self._create(vm_obj, self._skip_doms)
Line 240: 


-- 
To view, visit https://gerrit.ovirt.org/47246
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idd90750ab36cb90bad401fab0ff7ff98429e78a5
Gerrit-PatchSet: 9
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: automat...@ovirt.org
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to