Federico Simoncelli has posted comments on this change.
Change subject: domain: fix onDomainConnectivityStateChange attribute
......................................................................
Patch Set 7: (1 inline comment)
....................................................
File vdsm/storage/domainMonitor.py
Line 215: self.log.debug("Domain %s changed its status to %s",
self.sdUUID,
Line 216: "Valid" if self.nextStatus.valid else
"Invalid")
Line 217:
Line 218: try:
Line 219: self.domainMonitor().onDomainConnectivityStateChange \
I don't like one bit cluttering the env with temporary (unused) variables as a
workaround for long invocation lines. Second if self.domainMonitor is None it's
a big deal, it's not something we ever expect and it should be properly
reported. Are you suggesting to mask it with a different exception because QA
has an AttributeError grep on the logs? Basically you are asking to hide an
extremely dangerous situation that shouldn't happen. If it happens and we don't
have any feedback you'd be forcing someone to backtrace an unfinished domain
upgrade without giving any clue.
Line 220: .emit(self.sdUUID,
self.nextStatus.valid)
Line 221: except:
Line 222: self.log.warn("Could not emit domain state change
event",
Line 223: exc_info=True)
--
To view, visit http://gerrit.ovirt.org/7772
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If3b7e7c788b54726a3c820f4f2180884c74d93ef
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Eduardo <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Ryan Harper <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
