Bala.FA has posted comments on this change. Change subject: gluster: VDSM Gluster verbs to manage UFO/swift configuration ......................................................................
Patch Set 14: (4 inline comments) .................................................... File vdsm/gluster/cli.py Line 920: try: Line 921: config.readfp(open(config_file)) Line 922: except IOError as e: Line 923: errMsg = "%s: %s" % (e.errno, e.strerror) Line 924: raise ge.GlusterSwiftConfigOpenFailedException(err=[errMsg]) how about str(e)? Line 925: Line 926: return config Line 927: Line 928: Line 929: def _writeSwiftConfigFile(serverType, config): Line 930: config_file = SWIFT_CONFIG_FILES[serverType] Line 931: try: Line 932: # Write to a backup file Line 933: with open(config_file + ".bkp", 'wb') as configFile: I think creating a tempfile and write is better than '.bkp' Line 934: config.write(configFile) Line 935: Line 936: # Copy if write successful Line 937: os.rename(config_file + ".bkp", config_file) Line 933: with open(config_file + ".bkp", 'wb') as configFile: Line 934: config.write(configFile) Line 935: Line 936: # Copy if write successful Line 937: os.rename(config_file + ".bkp", config_file) Can we preserve original file? Line 938: except IOError as e: Line 939: errMsg = "%s: %s" % (e.errno, e.strerror) Line 940: raise ge.GlusterSwiftConfigWriteFailedException(err=[errMsg]) Line 941: Line 1014: config.set(section, configOption, unicode(value)) Line 1015: Line 1016: # Restore defaults Line 1017: config._defaults = defaultValues Line 1018: _writeSwiftConfigFile(serverType, config) I would like to keep these functions outside of cli.py and cli.py is meant for gluster cli bindings. -- To view, visit http://gerrit.ovirt.org/10864 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie966fb515275a0768f67cbbe2055a07002355327 Gerrit-PatchSet: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Aravinda VK <avish...@redhat.com> Gerrit-Reviewer: Aravinda VK <avish...@redhat.com> Gerrit-Reviewer: Bala.FA <barum...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Saggi Mizrahi <smizr...@redhat.com> Gerrit-Reviewer: Shireesh Anjal <san...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
