Change in vdsm[master]: New upgrade system
Giuseppe Vallarelli has posted comments on this change.
Change subject: New upgrade system
......................................................................
Patch Set 1:
(2 comments)
Some quick feedback even for a draft is always cool :)
....................................................
File vdsm/upgrade/upgrade.py
Line 40:
Line 41: def run(self):
Line 42: raise NotImplementedError
Line 43:
Line 44: def done(self):
done should not be part of the public api of the object, simply because you can call it
when after creating the object and that is not desirable. When you create an object with
some methods expect that anybody can call the methods in the order that he/she/it prefers.
I'm saying this because looking at the code it seems that done() only has sense after
a run().
I would change name of done to logRevision or something else, done is a very generic name
what is done?
Line 45: with open(self._getUpgradeFilePath(), 'w') as upgradeFile:
Line 46: upgradeFile.write(dsaversion.raw_version_revision)
Line 47:
Line 48: def _getUpgradeFilePath(self):
Line 45: with open(self._getUpgradeFilePath(), 'w') as upgradeFile:
Line 46: upgradeFile.write(dsaversion.raw_version_revision)
Line 47:
Line 48: def _getUpgradeFilePath(self):
Line 49: return constants.P_VDSM_LIB + "upgrade/%s" % self.upgradeName
By reading the current code you need an upgradeFilePath that you can build in the
constructor, _getUpgradeFilePath is not really useful right now.
I appreciate that you do command/query seperation.
--
To view, visit http://gerrit.ovirt.org/17726
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iba3c9c34f03134c192db1c2add31084824e195d9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Assaf Muller <amuller(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Giuseppe Vallarelli <gvallare(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes