Change in vdsm[master]: supervdsm: Fix race regarding instance ident
danken at redhat.com
danken at redhat.com
Thu Feb 14 20:15:00 UTC 2013
Dan Kenigsberg has posted comments on this change.
Change subject: supervdsm: Fix race regarding instance ident
......................................................................
Patch Set 3: I would prefer that you didn't submit this
(3 inline comments)
....................................................
File vdsm/supervdsm.py
Line 70: self._supervdsmProxy = supervdsmProxy
Line 71:
Line 72: def __call__(self, *args, **kwargs):
Line 73: def callMethod():
Line 74: instance = self._supervdsmProxy.getInstance()
Ok. For some reason you do not want to let an exception leak out of getInstance, an return None. I'm following.
Line 75: method = getattr(instance, self._funcName)
Line 76: return method(*args, **kwargs)
Line 77:
Line 78: if not self._supervdsmProxy.isRunning():
Line 71:
Line 72: def __call__(self, *args, **kwargs):
Line 73: def callMethod():
Line 74: instance = self._supervdsmProxy.getInstance()
Line 75: method = getattr(instance, self._funcName)
but now you get
AttributeError: 'NoneType' object has no attribute 'bla'
and I am lost. I what way is this exception better than the originally swallowed one?
Line 76: return method(*args, **kwargs)
Line 77:
Line 78: if not self._supervdsmProxy.isRunning():
Line 79: # getting inside only when svdsm is down. its rare case so we
Line 198: # We catch all exceptions as there is no documentation to what this
Line 199: # mehthod throws normally.
Line 200: except:
Line 201: self._log.warn("Could not get instance, this is perfectly fine "
Line 202: "since this method was created just to catch this "
I suppose I should have seen my name written in acrostichon or something?
Line 203: "exception and return None in case the instance "
Line 204: "does not exist. Some people fear that one day "
Line 205: "there might be a bug in multiprocessing that "
Line 206: "causes the method to throw an exception that is "
--
To view, visit http://gerrit.ovirt.org/11759
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4f548cb9ad805508cf75df691e0ab58b3278dc87
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Barak Azulay <bazulay at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Ohad Basan <obasan at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
More information about the vdsm-patches
mailing list