mooli tayer has posted comments on this change.

Change subject: vdsm-tool: suppoort dependencies between ModuleConfigure
......................................................................


Patch Set 2:

(4 comments)

http://gerrit.ovirt.org/#/c/31423/2/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:

Line 205:     sortedModules = []
Line 206:     while modulesNames:
Line 207:         foundNext = False
Line 208: 
Line 209:         for c in modulesNames:
> I do not understand this algorithm....
In f():

a dependent on c. b dependent on c. modules = a, b 

then we will end up with ret = [c, a, c, b]. No? 

Yes I go only forward: on each step I should always be able 

to select a module whose list of deps have all ready been met 

If not than I have a circle.

f does look simpler though.
Line 210:             requires = _getConfigurers()[c].getRequires()
Line 211:             if not requires or requires.issubset(includedNames):
Line 212:                 foundNext = True
Line 213:                 modulesNames.remove(c)


Line 207:         foundNext = False
Line 208: 
Line 209:         for c in modulesNames:
Line 210:             requires = _getConfigurers()[c].getRequires()
Line 211:             if not requires or requires.issubset(includedNames):
> empty set is always subset...
Right, Thanks.
Line 212:                 foundNext = True
Line 213:                 modulesNames.remove(c)
Line 214:                 sortedModules.append(c)
Line 215:                 includedNames.add(c)


Line 211:             if not requires or requires.issubset(includedNames):
Line 212:                 foundNext = True
Line 213:                 modulesNames.remove(c)
Line 214:                 sortedModules.append(c)
Line 215:                 includedNames.add(c)
> well,  I would have converted the sortedModules to set every time and drop 
I agree, thanks.
Line 216:                 break
Line 217: 
Line 218:         if not foundNext:
Line 219:             raise RuntimeError("Dependency circle found!")


Line 251:         args.modules = _getConfigurers().keys()
Line 252:     else:
Line 253:         _validateModulesNames(args.modules, parser.format_usage())
Line 254: 
Line 255:     args.modules = _sort_modules(_add_dependencies(args.modules))
> why can't the add_dependencies throw an exception if module is missing? rem
I do not mind catching a key error in add_dependencies.

I would still like to keep all the error formatting logic in 

a separate function though, and call it on Key error.
Line 256: 
Line 257:     args.modules = [_getConfigurers()[cName] for cName in 
args.modules]
Line 258: 


-- 
To view, visit http://gerrit.ovirt.org/31423
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibf63ce9aa3ca8edb82091d09b976e8e23896524e
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: mooli tayer <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to