On 09/27/2012 01:35 PM, Martin Kosek wrote:
A hotfix pushed in a scope of ticket 3088 forced conversion of DN
object (baseDN) in IPA client discovery so that ipa-client-install
does not crash when creating an IPA default.conf. Since this is not
a preferred way to handle DN objects, improve its usage:

- make sure, that baseDN retrieved by client discovery is always
   a DN object
- update ipachangeconf.py code to handle strings better and instead
   of concatenating objects, make sure they are converted to string
   first

As a side-effect of ipachangeconf changes, default.conf config file
generated by ipa-client-install has no longer empty new line at the
end of a file.

https://fedorahosted.org/freeipa/ticket/3088

ACK, but since you're reformatting the code, here are some minor issues found by the PEP8 checker.

diff --git a/ipa-client/ipaclient/ipachangeconf.py 
b/ipa-client/ipaclient/ipachangeconf.py
index 
f6288062be5c5d1b29341ed90814e1fa1431019c..2bb0cc487670ce74833fc09b06449d0a176ffaf5
 100644
--- a/ipa-client/ipaclient/ipachangeconf.py
+++ b/ipa-client/ipaclient/ipachangeconf.py
@@ -68,7 +68,7 @@ class IPAChangeConf:
          elif type(indent) is str:
              self.indent = (indent, )
          else:
-           raise ValueError, 'Indent must be a list of strings'
+           raise ValueError('Indent must be a list of strings')

Please re-indent to 4 spaces.


@@ -143,35 +143,50 @@ class IPAChangeConf:
      def getSectionLine(self, section):
          if len(self.sectnamdel) != 2:
              return section
-        return self.sectnamdel[0]+section+self.sectnamdel[1]+self.deol
+        return self._dump_line(self.sectnamdel[0],
+                               section,
+                               self.sectnamdel[1],
+                               self.deol)
+
+    def _dump_line(self, *args):
+        return u"".join(unicode(x) for x in args)

      def dump(self, options, level=0):
-        output = ""
+        output = []
          if level >= len(self.indent):
              level = len(self.indent)-1

          for o in options:
              if o['type'] == "section":
-                output += 
self.sectnamdel[0]+o['name']+self.sectnamdel[1]+self.deol
-                output += self.dump(o['value'], level+1)
+                output.append(self._dump_line(self.sectnamdel[0],
+                                              o['name'],
+                                              self.sectnamdel[1]))
+                output.append(self.dump(o['value'], level+1))

Please surround the + operator by spaces (here & below).

@@ -274,15 +297,19 @@ class IPAChangeConf:
                          opts.append(o)
                          continue
                      if no['action'] == 'comment':
-                       opts.append({'name':'comment', 'type':'comment',
-                                    
'value':self.dcomment+o['name']+self.dassign+o['value']})
+                        value = self._dump_line(self.dcomment,
+                                                o['name'],
+                                                self.dassign,
+                                                o['value'])
+                        opts.append({'name':'comment', 'type':'comment',
+                                     'value':value})

Please add a space after each colon.

--
PetrĀ³

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to