Change in vdsm[master]: [wip] Dynamic load of supervdsm_api funcs

edwardh at redhat.com edwardh at redhat.com
Mon Feb 1 09:17:34 UTC 2016


Edward Haas has posted comments on this change.

Change subject: [wip] Dynamic load of supervdsm_api funcs
......................................................................


Patch Set 3:

(3 comments)

https://gerrit.ovirt.org/#/c/52875/3/vdsm/supervdsmServer
File vdsm/supervdsmServer:

Line 162: getattr(m, func)
> not sure I understand ...
I meant to avoid the api bridging. I understand you prefer the api layer to be a bridge.


Line 152:         is_module = re.compile(r"^[^_].*\.pyc?$").search
        :         modules = set(os.path.splitext(name)[0]
        :                       for name in os.listdir(supervdsm_api.__path__[0])
        :                       if is_module(name))
        :         for module in modules:
        :             pkg = "%s.%s" % (supervdsm_api.__name__, module)
        :             m = __import__(pkg, globals(), locals(), [module], level=0)
        :             for func in dir(m):
        :                 if func.startswith('api_'):
        :                     # TODO: raise on duplications
        :                     self.api[func[4:]] = getattr(m, func)
> I thought about it , but I don't see how useful it his. its quite specific
It has functionality that has nothing to do with supervdsm, it just feels wrong to keep it here when some other api layer may want to use the same registration method of api/s.

Your call.


Line 160:                 if func.startswith('api_'):
Line 161:                     # TODO: raise on duplications
Line 162:                     self.api[func[4:]] = getattr(m, func)
Line 163: 
Line 164:     def __getattr__(self, name):
> it will try to look for the method under this class, and raise AttributeErr
We don't want to raise some other meaningful exception?
Line 165:         return self.api[name]
Line 166: 
Line 167:     @logDecorator
Line 168:     def getDevicePartedInfo(self, *args, **kwargs):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I67a1c349c15a03ba54788ffafd0c5b9d30ff7037
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list