Ayal Baron has posted comments on this change.
Change subject: gluster: VDSM Gluster verb to get swift configuration ......................................................................
Patch Set 20: I would prefer that you didn't submit this
(1 inline comment)
.................................................... File vdsm/gluster/swift.py Line 100: Line 101: return configValues Line 102: Line 103: Line 104: @exportToSuperVdsm I don't see why this module is glusterswift specific. All the code here is general config file parsing. There are 2 things that are specific here: 1. the config file paths - this could be handled in a wrapper (upper layer). In fact, this is tying specific vdsm version to specific gluster-swift version since if you change path to config files or add new ones, you'd have to update this code. Since this is generic config parsing, I'm not sure why we'd want to hard code the config file names here (in which case this entire patch should be generic config file parsing and not gluster-swift specific at all) 2. the exceptions thrown - I see no reason for these to be this specific actually. When I request a specific config file and specific section and get a 'InvalidSectionException' I know exactly what the problem is. Line 105: def swiftConfigGet(serverType, section=None, configOption=None):
-- 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: 20 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Aravinda VK avishwan@redhat.com Gerrit-Reviewer: Aravinda VK avishwan@redhat.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Bala.FA barumuga@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shireesh Anjal sanjal@redhat.com Gerrit-Reviewer: Timothy Asir tjeyasin@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server