Change in vdsm[master]: Adding supervdsm uts

agl at us.ibm.com agl at us.ibm.com
Wed Oct 17 19:37:57 UTC 2012


Adam Litke has posted comments on this change.

Change subject: Adding supervdsm uts
......................................................................


Patch Set 3: (3 inline comments)

....................................................
Commit Message
Line 4: Commit:     Yaniv Bronhaim <bronhaim at gmail.com>
Line 5: CommitDate: 2012-10-17 18:33:58 +0200
Line 6: 
Line 7: Adding supervdsm uts
Line 8: 
what is 'uts'?  Perhaps you could state the rough nature of testing that you are adding.  Mention how the tests work (ie. overriding the pid file used by the test server, etc).  It would be nice if, when browsing the git log, I can tell a little bit about a patch from the commit message.  This message tells me absolutely nothing.
Line 9: Change-Id: I08880b12db8a6ca9cc57d74d2ef2a86980a5b097


....................................................
File vdsm/supervdsm.py
Line 92:     def __init__(self):
Line 93:         self._firstLaunch = True
Line 94:         self.pidfile = PIDFILE
Line 95:         self.timestamp = TIMESTAMP
Line 96:         self.address = ADDRESS
Something like this:

##
# These variables are overridden during testing.  If you make changes to them
# be careful to maintain the option of creating a test instance
##

If you are opposed to a comment, then you should add a new class method to override these variables.  By overriding these variables you have created a new interface to this class.  What I am asking for is that you make that interface explicit so we don't accidentally break it in the future.
Line 97: 
Line 98:     def open(self, *args, **kwargs):
Line 99:         return self._manager.open(*args, **kwargs)
Line 100: 


Line 165:         self._log.debug("Trying to connect to Super Vdsm")
Line 166:         try:
Line 167:             self._manager.connect()
Line 168:         except Exception, ex:
Line 169:             self._log.debug("Connect to %s failed %s", self.address, ex)
Ok, then keep the message verbosity as warn too.  You can change it to debug in your later patch.
Line 170:             raise
Line 171:         self._svdsm = self._manager.instance()
Line 172: 
Line 173:     def launch(self):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I08880b12db8a6ca9cc57d74d2ef2a86980a5b097
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Adam Litke <agl at us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>


More information about the vdsm-patches mailing list