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

edwardh at redhat.com edwardh at redhat.com
Mon Feb 1 07:53:00 UTC 2016


Edward Haas has posted comments on this change.

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


Patch Set 3:

(4 comments)

I like++

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

Line 162: getattr(m, func)
What about removing one hop here and returning the module func itself.
The api definitions will be something like this:
def api_foo()
    return module.foo

Where 'foo' is the method of the module.


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)
Looks generic enough to be useful for others, can we locate it under its own file and input its variables?

It would also be nice to make it a bit more readable:
for modules in _get_modules(path):
    m = _import_module(path, module)
    self.api.update(_get_funcs(m, 'api_'))


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):
What if the api is not available?
What happens today?

As all api/s use logDecorator, can we call it from here instead?
Line 165:         return self.api[name]
Line 166: 
Line 167:     @logDecorator
Line 168:     def getDevicePartedInfo(self, *args, **kwargs):


https://gerrit.ovirt.org/#/c/52875/3/vdsm/supervdsm_api/__init__.py
File vdsm/supervdsm_api/__init__.py:

Maybe a remark on why it is needed?


-- 
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