URL: https://github.com/freeipa/freeipa/pull/97
Author: mbasti-rh
 Title: #97: Pylint fixes
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/97/head:pr97
git checkout pr97
From fc4c31912df4d004f04700fa2a4f4efceb144247 Mon Sep 17 00:00:00 2001
From: Jan Barta <55042ba...@sstebrno.eu>
Date: Thu, 2 Jun 2016 09:58:52 +0200
Subject: [PATCH 01/11] pylint: fix simplifiable-if-statement warnings

fix inefficient if statements, enable pylint check
---
 ipalib/config.py                   |  8 ++------
 ipaplatform/base/services.py       | 12 ++++--------
 ipapython/ipautil.py               | 10 ++--------
 ipapython/nsslib.py                |  5 +----
 ipapython/sysrestore.py            |  5 +----
 ipaserver/install/ca.py            |  7 +------
 ipaserver/plugins/dogtag.py        | 10 ++--------
 ipatests/test_integration/tasks.py | 12 ++----------
 pylintrc                           |  1 -
 9 files changed, 15 insertions(+), 55 deletions(-)

diff --git a/ipalib/config.py b/ipalib/config.py
index 55a95bb..eb6c3ae 100644
--- a/ipalib/config.py
+++ b/ipalib/config.py
@@ -455,14 +455,10 @@ def _bootstrap(self, **overrides):
 
         # Determine if running in source tree:
         if 'in_tree' not in self:
-            if (
+            self.in_tree = (
                 self.bin == self.site_packages
                 and path.isfile(path.join(self.bin, 'setup.py'))
-            ):
-                self.in_tree = True
-            else:
-                self.in_tree = False
-
+            )
         if self.in_tree and 'mode' not in self:
             self.mode = 'developer'
 
diff --git a/ipaplatform/base/services.py b/ipaplatform/base/services.py
index a36b2f4..750d979 100644
--- a/ipaplatform/base/services.py
+++ b/ipaplatform/base/services.py
@@ -271,10 +271,8 @@ def stop(self, instance_name="", capture_output=True):
 
         ipautil.run(args, skip_output=not capture_output)
 
-        if getattr(self.api.env, 'context', None) in ['ipactl', 'installer']:
-            update_service_list = True
-        else:
-            update_service_list = False
+        update_service_list = getattr(self.api.env, 'context',
+                                      None) in ['ipactl', 'installer']
         super(SystemdService, self).stop(
             instance_name,
             update_service_list=update_service_list)
@@ -284,10 +282,8 @@ def start(self, instance_name="", capture_output=True, wait=True):
                      self.service_instance(instance_name)],
                     skip_output=not capture_output)
 
-        if getattr(self.api.env, 'context', None) in ['ipactl', 'installer']:
-            update_service_list = True
-        else:
-            update_service_list = False
+        update_service_list = getattr(self.api.env, 'context',
+                                      None) in ['ipactl', 'installer']
 
         if wait and self.is_running(instance_name):
             self.wait_for_open_ports(self.service_instance(instance_name))
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 64901b5..62d029d 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -519,20 +519,14 @@ def nolog_replace(string, nolog):
 def file_exists(filename):
     try:
         mode = os.stat(filename)[stat.ST_MODE]
-        if stat.S_ISREG(mode):
-            return True
-        else:
-            return False
+        return bool(stat.S_ISREG(mode))
     except Exception:
         return False
 
 def dir_exists(filename):
     try:
         mode = os.stat(filename)[stat.ST_MODE]
-        if stat.S_ISDIR(mode):
-            return True
-        else:
-            return False
+        return bool(stat.S_ISDIR(mode))
     except Exception:
         return False
 
diff --git a/ipapython/nsslib.py b/ipapython/nsslib.py
index b5e5b65..1573de9 100644
--- a/ipapython/nsslib.py
+++ b/ipapython/nsslib.py
@@ -74,10 +74,7 @@ def auth_certificate_callback(sock, check_sig, is_server, certdb):
                               ', '.join(nss.cert_usage_flags(intended_usage)))
 
     # Is the intended usage a proper subset of the approved usage
-    if approved_usage & intended_usage:
-        cert_is_valid = True
-    else:
-        cert_is_valid = False
+    cert_is_valid = bool(approved_usage & intended_usage)
 
     # If this is a server, we're finished
     if is_server or not cert_is_valid:
diff --git a/ipapython/sysrestore.py b/ipapython/sysrestore.py
index e0d0908..cd09cae 100644
--- a/ipapython/sysrestore.py
+++ b/ipapython/sysrestore.py
@@ -437,7 +437,4 @@ def has_state(self, module):
         Can be used to determine if a service is configured.
         """
 
-        if module in self.modules:
-            return True
-        else:
-            return False
+        return module in self.modules
diff --git a/ipaserver/install/ca.py b/ipaserver/install/ca.py
index 00e0b03..dcda19a 100644
--- a/ipaserver/install/ca.py
+++ b/ipaserver/install/ca.py
@@ -127,14 +127,9 @@ def install_step_0(standalone, replica_config, options):
     if replica_config is not None:
         # Configure the CA if necessary
         if standalone:
-            postinstall = True
-        else:
-            postinstall = False
-
-        if standalone:
             api.Backend.ldap2.disconnect()
 
-        cainstance.install_replica_ca(replica_config, postinstall,
+        cainstance.install_replica_ca(replica_config, standalone,
                 ra_p12=getattr(options, 'ra_p12', None))
 
         if standalone and not api.Backend.ldap2.isconnected():
diff --git a/ipaserver/plugins/dogtag.py b/ipaserver/plugins/dogtag.py
index 644b41e..ffe6ead 100644
--- a/ipaserver/plugins/dogtag.py
+++ b/ipaserver/plugins/dogtag.py
@@ -1750,10 +1750,7 @@ def revoke_certificate(self, serial_number, revocation_reason=0):
         # Return command result
         cmd_result = {}
 
-        if parse_result.get('revoked') == 'yes':
-            cmd_result['revoked'] = True
-        else:
-            cmd_result['revoked'] = False
+        cmd_result['revoked'] = parse_result.get('revoked') == 'yes'
 
         return cmd_result
 
@@ -1814,10 +1811,7 @@ def take_certificate_off_hold(self, serial_number):
         if 'error_string' in parse_result:
             cmd_result['error_string'] = parse_result['error_string']
 
-        if parse_result.get('unrevoked') == 'yes':
-            cmd_result['unrevoked'] = True
-        else:
-            cmd_result['unrevoked'] = False
+        cmd_result['unrevoked'] = parse_result.get('unrevoked') == 'yes'
 
         return cmd_result
 
diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index ac36e2e..0c3e140 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -172,11 +172,7 @@ def host_service_active(host, service):
     res = host.run_command(['systemctl', 'is-active', '--quiet', service],
                            raiseonerr=False)
 
-    if res.returncode == 0:
-        return True
-    else:
-        return False
-
+    return res.returncode == 0
 
 def fix_apache_semaphores(master):
     systemd_available = master.transport.file_exists(paths.SYSTEMCTL)
@@ -321,11 +317,7 @@ def master_authoritative_for_client_domain(master, client):
     zone = ".".join(client.hostname.split('.')[1:])
     result = master.run_command(["ipa", "dnszone-show", zone],
                                 raiseonerr=False)
-    if result.returncode == 0:
-        return True
-    else:
-        return False
-
+    return result.returncode == 0
 
 def replica_prepare(master, replica):
     fix_apache_semaphores(replica)
diff --git a/pylintrc b/pylintrc
index bb9c636..1f7e49f 100644
--- a/pylintrc
+++ b/pylintrc
@@ -23,7 +23,6 @@ disable=
     interface-not-implemented,
     no-self-use,
     redefined-variable-type,
-    simplifiable-if-statement,
     too-few-public-methods,
     too-many-ancestors,
     too-many-arguments,

From 08b07f42b7565a29e69c6ae7cdd868d9fe1a47eb Mon Sep 17 00:00:00 2001
From: Jan Barta <55042ba...@sstebrno.eu>
Date: Fri, 3 Jun 2016 10:05:34 +0200
Subject: [PATCH 02/11] pylint: fix unneeded-not

---
 ipalib/aci.py                               | 6 +++---
 ipalib/parameters.py                        | 4 ++--
 ipaserver/plugins/baseldap.py               | 2 +-
 ipaserver/plugins/delegation.py             | 2 +-
 ipaserver/plugins/group.py                  | 4 ++--
 ipaserver/plugins/host.py                   | 2 +-
 ipaserver/plugins/selfservice.py            | 2 +-
 ipatests/test_ipalib/test_aci.py            | 2 +-
 ipatests/test_webui/ui_driver.py            | 2 +-
 ipatests/test_xmlrpc/tracker/user_plugin.py | 2 +-
 pylintrc                                    | 1 -
 11 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/ipalib/aci.py b/ipalib/aci.py
index a76435f..97ed0c3 100755
--- a/ipalib/aci.py
+++ b/ipalib/aci.py
@@ -156,10 +156,10 @@ def validate(self):
 
            returns True if valid
         """
-        if not type(self.permissions) in (tuple, list):
+        if type(self.permissions) not in (tuple, list):
             raise SyntaxError("permissions must be a list")
         for p in self.permissions:
-            if not p.lower() in PERMISSIONS:
+            if p.lower() not in PERMISSIONS:
                 raise SyntaxError("invalid permission: '%s'" % p)
         if not self.name:
             raise SyntaxError("name must be set")
@@ -185,7 +185,7 @@ def set_target_attr(self, attr, operator="="):
             if 'targetattr' in self.target:
                 del self.target['targetattr']
             return
-        if not type(attr) in (tuple, list):
+        if type(attr) not in (tuple, list):
             attr = [attr]
         self.target['targetattr'] = {}
         self.target['targetattr']['expression'] = attr
diff --git a/ipalib/parameters.py b/ipalib/parameters.py
index 37f9650..6a289ac 100644
--- a/ipalib/parameters.py
+++ b/ipalib/parameters.py
@@ -434,9 +434,9 @@ def __init__(self, name, *rules, **kw):
         # Merge in kw from parse_param_spec():
         (name, kw_from_spec) = parse_param_spec(name)
         check_name(name)
-        if not 'required' in kw:
+        if 'required' not in kw:
             kw['required'] = kw_from_spec['required']
-        if not 'multivalue' in kw:
+        if 'multivalue' not in kw:
             kw['multivalue'] = kw_from_spec['multivalue']
 
         # Add 'default' to self.kwargs
diff --git a/ipaserver/plugins/baseldap.py b/ipaserver/plugins/baseldap.py
index f7844e3..1df8da4 100644
--- a/ipaserver/plugins/baseldap.py
+++ b/ipaserver/plugins/baseldap.py
@@ -909,7 +909,7 @@ def _convert_2_dict(self, ldap, attrs):
         newdict = {}
         if attrs is None:
             attrs = []
-        elif not type(attrs) in (list, tuple):
+        elif type(attrs) not in (list, tuple):
             attrs = [attrs]
         for a in attrs:
             m = re.match("\s*(.*?)\s*=\s*(.*?)\s*$", a)
diff --git a/ipaserver/plugins/delegation.py b/ipaserver/plugins/delegation.py
index 6340d1a..28a8f30 100644
--- a/ipaserver/plugins/delegation.py
+++ b/ipaserver/plugins/delegation.py
@@ -132,7 +132,7 @@ class delegation_add(crud.Create):
     msg_summary = _('Added delegation "%(value)s"')
 
     def execute(self, aciname, **kw):
-        if not 'permissions' in kw:
+        if 'permissions' not in kw:
             kw['permissions'] = (u'write',)
         kw['aciprefix'] = ACI_PREFIX
         result = api.Command['aci_add'](aciname, **kw)['result']
diff --git a/ipaserver/plugins/group.py b/ipaserver/plugins/group.py
index 6677634..115db9d 100644
--- a/ipaserver/plugins/group.py
+++ b/ipaserver/plugins/group.py
@@ -314,7 +314,7 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
                 raise errors.MutuallyExclusiveError(reason=_('gid cannot be set for external group'))
         elif not options['nonposix']:
             entry_attrs['objectclass'].append('posixgroup')
-            if not 'gidnumber' in options:
+            if 'gidnumber' not in options:
                 entry_attrs['gidnumber'] = baseldap.DNA_MAGIC
         return dn
 
@@ -395,7 +395,7 @@ def pre_callback(self, ldap, dn, entry_attrs, *keys, **options):
             else:
                 old_entry_attrs['objectclass'].append('posixgroup')
                 entry_attrs['objectclass'] = old_entry_attrs['objectclass']
-                if not 'gidnumber' in options:
+                if 'gidnumber' not in options:
                     entry_attrs['gidnumber'] = baseldap.DNA_MAGIC
 
         if options['external']:
diff --git a/ipaserver/plugins/host.py b/ipaserver/plugins/host.py
index 2362b62..d3e3c27 100644
--- a/ipaserver/plugins/host.py
+++ b/ipaserver/plugins/host.py
@@ -666,7 +666,7 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
                     options['ip_address'],
                     check_forward=True,
                     check_reverse=check_reverse)
-        if not options.get('force', False) and not 'ip_address' in options:
+        if not options.get('force', False) and 'ip_address' not in options:
             util.verify_host_resolvable(keys[-1])
         if 'locality' in entry_attrs:
             entry_attrs['l'] = entry_attrs['locality']
diff --git a/ipaserver/plugins/selfservice.py b/ipaserver/plugins/selfservice.py
index 9697493..9009b7a 100644
--- a/ipaserver/plugins/selfservice.py
+++ b/ipaserver/plugins/selfservice.py
@@ -124,7 +124,7 @@ class selfservice_add(crud.Create):
     msg_summary = _('Added selfservice "%(value)s"')
 
     def execute(self, aciname, **kw):
-        if not 'permissions' in kw:
+        if 'permissions' not in kw:
             kw['permissions'] = (u'write',)
         kw['selfaci'] = True
         kw['aciprefix'] = ACI_PREFIX
diff --git a/ipatests/test_ipalib/test_aci.py b/ipatests/test_ipalib/test_aci.py
index 8ced2a9..5ce23db 100644
--- a/ipatests/test_ipalib/test_aci.py
+++ b/ipatests/test_ipalib/test_aci.py
@@ -94,7 +94,7 @@ def test_aci_equality():
 
     assert a.isequal(b)
     assert a == b
-    assert not a != b
+    assert not a != b  # pylint: disable=unneeded-not
 
 
 def check_aci_inequality(b):
diff --git a/ipatests/test_webui/ui_driver.py b/ipatests/test_webui/ui_driver.py
index 4bbb90c..ab917d8 100644
--- a/ipatests/test_webui/ui_driver.py
+++ b/ipatests/test_webui/ui_driver.py
@@ -183,7 +183,7 @@ def get_driver(self):
             options.binary_location = paths.CHROMIUM_BROWSER
 
         if driver_type == 'remote':
-            if not 'host' in self.config:
+            if 'host' not in self.config:
                 raise nose.SkipTest('Selenium server host not configured')
             host = self.config["host"]
 
diff --git a/ipatests/test_xmlrpc/tracker/user_plugin.py b/ipatests/test_xmlrpc/tracker/user_plugin.py
index b55deb9..6bc0ce1 100644
--- a/ipatests/test_xmlrpc/tracker/user_plugin.py
+++ b/ipatests/test_xmlrpc/tracker/user_plugin.py
@@ -172,7 +172,7 @@ def track_create(self):
                          self.api.env.realm)
                     )]
             else:
-                if not type(self.kwargs[key]) is list:
+                if type(self.kwargs[key]) is not list:
                     self.attrs[key] = [self.kwargs[key]]
                 else:
                     self.attrs[key] = self.kwargs[key]
diff --git a/pylintrc b/pylintrc
index 1f7e49f..140621e 100644
--- a/pylintrc
+++ b/pylintrc
@@ -84,7 +84,6 @@ disable=
     not-an-iterable,
     singleton-comparison,
     misplaced-comparison-constant,
-    unneeded-not,
     not-a-mapping,
     singleton-comparison
 

From 145ea03c92011ca99353018f2563151c1d6fb499 Mon Sep 17 00:00:00 2001
From: Jan Barta <55042ba...@sstebrno.eu>
Date: Fri, 3 Jun 2016 10:47:39 +0200
Subject: [PATCH 03/11] pylint: fix pointless-statement

---
 ipatests/test_ipapython/test_dn.py      | 22 +++++++++++-----------
 ipatests/test_ipapython/test_ipautil.py |  2 +-
 pylintrc                                |  1 -
 3 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/ipatests/test_ipapython/test_dn.py b/ipatests/test_ipapython/test_dn.py
index e1784bc..e09186f 100644
--- a/ipatests/test_ipapython/test_dn.py
+++ b/ipatests/test_ipapython/test_dn.py
@@ -138,10 +138,10 @@ def test_indexing(self):
         self.assertEqual(ava1[1], self.value1)
 
         with self.assertRaises(KeyError):
-            ava1['foo']
+            ava1['foo']  # pylint: disable=pointless-statement
 
         with self.assertRaises(KeyError):
-            ava1[3]
+            ava1[3]  # pylint: disable=pointless-statement
 
     def test_properties(self):
         ava1 = AVA(self.ava1)
@@ -500,25 +500,25 @@ def test_indexing(self):
         self.assertEqual(rdn1[0], self.ava1)
         self.assertEqual(rdn1[self.ava1.attr], self.ava1.value)
         with self.assertRaises(KeyError):
-            rdn1['foo']
+            rdn1['foo']  # pylint: disable=pointless-statement
 
         self.assertEqual(rdn2[0], self.ava2)
         self.assertEqual(rdn2[self.ava2.attr], self.ava2.value)
         with self.assertRaises(KeyError):
-            rdn2['foo']
+            rdn2['foo']  # pylint: disable=pointless-statement
 
         self.assertEqual(rdn3[0], self.ava1)
         self.assertEqual(rdn3[self.ava1.attr], self.ava1.value)
         self.assertEqual(rdn3[1], self.ava2)
         self.assertEqual(rdn3[self.ava2.attr], self.ava2.value)
         with self.assertRaises(KeyError):
-            rdn3['foo']
+            rdn3['foo']  # pylint: disable=pointless-statement
 
         self.assertEqual(rdn1.attr, self.attr1)
         self.assertEqual(rdn1.value, self.value1)
 
         with self.assertRaises(TypeError):
-            rdn3[1.0]
+            rdn3[1.0]  # pylint: disable=pointless-statement
 
         # Slices
         self.assertEqual(rdn3[0:1], [self.ava1])
@@ -915,22 +915,22 @@ def test_indexing(self):
         self.assertEqual(dn1[0], self.rdn1)
         self.assertEqual(dn1[self.rdn1.attr], self.rdn1.value)
         with self.assertRaises(KeyError):
-            dn1['foo']
+            dn1['foo']  # pylint: disable=pointless-statement
 
         self.assertEqual(dn2[0], self.rdn2)
         self.assertEqual(dn2[self.rdn2.attr], self.rdn2.value)
         with self.assertRaises(KeyError):
-            dn2['foo']
+            dn2['foo']  # pylint: disable=pointless-statement
 
         self.assertEqual(dn3[0], self.rdn1)
         self.assertEqual(dn3[self.rdn1.attr], self.rdn1.value)
         self.assertEqual(dn3[1], self.rdn2)
         self.assertEqual(dn3[self.rdn2.attr], self.rdn2.value)
         with self.assertRaises(KeyError):
-            dn3['foo']
+            dn3['foo']  # pylint: disable=pointless-statement
 
         with self.assertRaises(TypeError):
-            dn3[1.0]
+            dn3[1.0]  # pylint: disable=pointless-statement
 
     def test_assignments(self):
         dn = dn2 = DN('t=0,t=1,t=2,t=3,t=4,t=5,t=6,t=7,t=8,t=9')
@@ -1102,7 +1102,7 @@ def test_replace(self):
         # pylint: disable=no-member
         dn = DN('t=0,t=1,t=2,t=3,t=4,t=5,t=6,t=7,t=8,t=9')
         with self.assertRaises(AttributeError):
-            dn.replace
+            dn.replace  # pylint: disable=pointless-statement
 
     def test_hashing(self):
         # create DN's that are equal but differ in case
diff --git a/ipatests/test_ipapython/test_ipautil.py b/ipatests/test_ipapython/test_ipautil.py
index be59665..d0a68df 100644
--- a/ipatests/test_ipapython/test_ipautil.py
+++ b/ipatests/test_ipapython/test_ipautil.py
@@ -109,7 +109,7 @@ def test_getitem(self):
         nose.tools.assert_equal("VAL3", self.cidict["key3"])
         nose.tools.assert_equal("VAL3", self.cidict["KEY3"])
         with nose.tools.assert_raises(KeyError):
-            self.cidict["key4"]
+            self.cidict["key4"]  # pylint: disable=pointless-statement
 
     def test_get(self):
         nose.tools.assert_equal("val1", self.cidict.get("Key1"))
diff --git a/pylintrc b/pylintrc
index 140621e..40a8271 100644
--- a/pylintrc
+++ b/pylintrc
@@ -49,7 +49,6 @@ disable=
     global-statement,
     global-variable-not-assigned,
     no-init,
-    pointless-statement,
     pointless-string-statement,
     protected-access,
     redefine-in-handler,

From 1a809672aa0293009013e78bfc9e63880689917c Mon Sep 17 00:00:00 2001
From: Jan Barta <55042ba...@sstebrno.eu>
Date: Fri, 3 Jun 2016 11:09:00 +0200
Subject: [PATCH 04/11] pylint: fix redefine-in-handler

---
 install/tools/ipa-csreplica-manage | 14 +++++++-------
 ipaserver/plugins/baseldap.py      | 10 +++++-----
 pylintrc                           |  1 -
 3 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/install/tools/ipa-csreplica-manage b/install/tools/ipa-csreplica-manage
index a0a61b5..2a2f2ae 100755
--- a/install/tools/ipa-csreplica-manage
+++ b/install/tools/ipa-csreplica-manage
@@ -190,9 +190,9 @@ def del_link(realm, replica1, replica2, dirman_passwd, force=False):
 
         # Find the DN of the replication agreement to remove
         replica2_dn = None
-        for e in repl_list:
-            if e.single_value.get('nsDS5ReplicaHost') == replica1:
-                replica2_dn = e.dn
+        for entry in repl_list:
+            if entry.single_value.get('nsDS5ReplicaHost') == replica1:
+                replica2_dn = entry.dn
                 break
 
         # This should never happen
@@ -203,8 +203,8 @@ def del_link(realm, replica1, replica2, dirman_passwd, force=False):
         print("'%s' has no replication agreement for '%s'" % (replica2, replica1))
         if not force:
             return
-    except Exception as e:
-        print("Failed to get data from '%s': %s" % (replica2, e))
+    except Exception as exc:
+        print("Failed to get data from '%s': %s" % (replica2, exc))
         if not force:
             sys.exit(1)
 
@@ -213,8 +213,8 @@ def del_link(realm, replica1, replica2, dirman_passwd, force=False):
         try:
             repl2.delete_agreement(replica1, replica2_dn)
             repl2.delete_referral(replica1, repl1.port)
-        except Exception as e:
-            print("Unable to remove agreement on %s: %s" % (replica2, e))
+        except Exception as exc:
+            print("Unable to remove agreement on %s: %s" % (replica2, exc))
             failed = True
 
         if failed:
diff --git a/ipaserver/plugins/baseldap.py b/ipaserver/plugins/baseldap.py
index 1df8da4..e2669ca 100644
--- a/ipaserver/plugins/baseldap.py
+++ b/ipaserver/plugins/baseldap.py
@@ -2035,9 +2035,9 @@ def sort_key(x):
                 entries.sort(key=sort_key)
 
         if not options.get('raw', False):
-            for e in entries:
-                self.obj.get_indirect_members(e, attrs_list)
-                self.obj.convert_attribute_members(e, *args, **options)
+            for entry in entries:
+                self.obj.get_indirect_members(entry, attrs_list)
+                self.obj.convert_attribute_members(entry, *args, **options)
 
         for (i, e) in enumerate(entries):
             entries[i] = entry_to_dict(e, **options)
@@ -2051,9 +2051,9 @@ def sort_key(x):
 
         try:
             ldap.handle_truncated_result(truncated)
-        except errors.LimitsExceeded as e:
+        except errors.LimitsExceeded as exc:
             add_message(options['version'], result, SearchResultTruncated(
-                reason=e))
+                reason=exc))
 
         return result
 
diff --git a/pylintrc b/pylintrc
index 40a8271..0b04b01 100644
--- a/pylintrc
+++ b/pylintrc
@@ -51,7 +51,6 @@ disable=
     no-init,
     pointless-string-statement,
     protected-access,
-    redefine-in-handler,
     redefined-builtin,
     redefined-outer-name,
     super-init-not-called,

From beb6c2fa884b60da870fc32c53f0d78e70d43b14 Mon Sep 17 00:00:00 2001
From: Jan Barta <55042ba...@sstebrno.eu>
Date: Fri, 3 Jun 2016 12:45:01 +0200
Subject: [PATCH 05/11] pylint: fix old-style-class

---
 ipaclient/ipachangeconf.py        | 2 +-
 ipalib/aci.py                     | 3 ++-
 ipapython/config.py               | 3 ++-
 ipapython/graph.py                | 2 +-
 ipapython/sysrestore.py           | 6 ++++--
 ipaserver/install/installutils.py | 2 +-
 ipaserver/install/ldapupdate.py   | 2 +-
 pylintrc                          | 1 -
 8 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/ipaclient/ipachangeconf.py b/ipaclient/ipachangeconf.py
index 1b5167c..64177f8 100644
--- a/ipaclient/ipachangeconf.py
+++ b/ipaclient/ipachangeconf.py
@@ -48,7 +48,7 @@ def openLocked(filename, perms):
     #TODO: put section delimiters as separating element of the list
     #      so that we can process multiple sections in one go
     #TODO: add a comment all but provided options as a section option
-class IPAChangeConf:
+class IPAChangeConf(object):
 
     def __init__(self, name):
         self.progname = name
diff --git a/ipalib/aci.py b/ipalib/aci.py
index 97ed0c3..da082ae 100755
--- a/ipalib/aci.py
+++ b/ipalib/aci.py
@@ -39,7 +39,8 @@
 PERMISSIONS = ["read", "write", "add", "delete", "search", "compare",
                "selfwrite", "proxy", "all"]
 
-class ACI:
+
+class ACI(object):
     """
     Holds the basic data for an ACI entry, as stored in the cn=accounts
     entry in LDAP.  Has methods to parse an ACI string and export to an
diff --git a/ipapython/config.py b/ipapython/config.py
index 70afaff..38e1087 100644
--- a/ipapython/config.py
+++ b/ipapython/config.py
@@ -126,7 +126,8 @@ def verify_args(parser, args, needed_args = None):
     elif len_have < len_need:
         parser.error("no %s specified" % needed_list[len_have])
 
-class IPAConfig:
+
+class IPAConfig(object):
     def __init__(self):
         self.default_realm = None
         self.default_server = []
diff --git a/ipapython/graph.py b/ipapython/graph.py
index b4373ab..0e06a08 100644
--- a/ipapython/graph.py
+++ b/ipapython/graph.py
@@ -3,7 +3,7 @@
 #
 
 
-class Graph():
+class Graph(object):
     """
     Simple oriented graph structure
 
diff --git a/ipapython/sysrestore.py b/ipapython/sysrestore.py
index cd09cae..1cc7ce5 100644
--- a/ipapython/sysrestore.py
+++ b/ipapython/sysrestore.py
@@ -42,7 +42,8 @@
 SYSRESTORE_INDEXFILE = "sysrestore.index"
 SYSRESTORE_STATEFILE = "sysrestore.state"
 
-class FileStore:
+
+class FileStore(object):
     """Class for handling backup and restore of files"""
 
     def __init__(self, path = SYSRESTORE_PATH, index_file = SYSRESTORE_INDEXFILE):
@@ -290,7 +291,8 @@ def untrack_file(self, path):
 
         return True
 
-class StateFile:
+
+class StateFile(object):
     """A metadata file for recording system state which can
     be backed up and later restored.
     StateFile gets reloaded every time to prevent loss of information
diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index 7578bf8..bf179a2 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -102,7 +102,7 @@ class UpgradeMissingVersionError(UpgradeVersionError):
     pass
 
 
-class ReplicaConfig:
+class ReplicaConfig(object):
     def __init__(self, top_dir=None):
         self.realm_name = ""
         self.domain_name = ""
diff --git a/ipaserver/install/ldapupdate.py b/ipaserver/install/ldapupdate.py
index 1b39745..19343cc 100644
--- a/ipaserver/install/ldapupdate.py
+++ b/ipaserver/install/ldapupdate.py
@@ -134,7 +134,7 @@ def safe_output(attr, values):
     return values
 
 
-class LDAPUpdate:
+class LDAPUpdate(object):
     action_keywords = ["default", "add", "remove", "only", "onlyifexist", "deleteentry", "replace", "addifnew", "addifexist"]
 
     def __init__(self, dm_password=None, sub_dict={},
diff --git a/pylintrc b/pylintrc
index 0b04b01..102d2d8 100644
--- a/pylintrc
+++ b/pylintrc
@@ -69,7 +69,6 @@ disable=
     line-too-long,
     missing-docstring,
     multiple-statements,
-    old-style-class,
     superfluous-parens,
     too-many-lines,
     unidiomatic-typecheck,

From 0dff23da8b722bd2c45ba488577c4c89fe676b54 Mon Sep 17 00:00:00 2001
From: Jan Barta <55042ba...@sstebrno.eu>
Date: Fri, 3 Jun 2016 12:52:54 +0200
Subject: [PATCH 06/11] pylint: fix bad-classmethod-argument

---
 ipatests/test_integration/test_caless.py | 4 ++--
 ipatests/test_ipalib/test_rpc.py         | 4 ++--
 pylintrc                                 | 1 -
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/ipatests/test_integration/test_caless.py b/ipatests/test_integration/test_caless.py
index c9d9033..5c77c87 100644
--- a/ipatests/test_integration/test_caless.py
+++ b/ipatests/test_integration/test_caless.py
@@ -167,9 +167,9 @@ def copy_cert(cls, host, filename):
                                 os.path.join(host.config.test_dir, filename))
 
     @classmethod
-    def uninstall_server(self, host=None):
+    def uninstall_server(cls, host=None):
         if host is None:
-            host = self.master
+            host = cls.master
         host.run_command(['ipa-server-install', '--uninstall', '-U'])
 
     def prepare_replica(self, _replica_number=0, replica=None, master=None,
diff --git a/ipatests/test_ipalib/test_rpc.py b/ipatests/test_ipalib/test_rpc.py
index f038951..ae83a5d 100644
--- a/ipatests/test_ipalib/test_rpc.py
+++ b/ipatests/test_ipalib/test_rpc.py
@@ -257,7 +257,7 @@ class user_add(Command):
 
 class test_xml_introspection(object):
     @classmethod
-    def setup_class(self):
+    def setup_class(cls):
         try:
             api.Backend.xmlclient.connect()
         except (errors.NetworkError, IOError):
@@ -265,7 +265,7 @@ def setup_class(self):
                                 (__name__, api.env.xmlrpc_uri))
 
     @classmethod
-    def teardown_class(self):
+    def teardown_class(cls):
         request.destroy_context()
 
     def test_list_methods(self):
diff --git a/pylintrc b/pylintrc
index 102d2d8..115b295 100644
--- a/pylintrc
+++ b/pylintrc
@@ -59,7 +59,6 @@ disable=
     unused-argument,
     unused-variable,
     useless-else-on-loop,
-    bad-classmethod-argument,
     bad-continuation,
     bad-mcs-classmethod-argument,
     bad-mcs-method-argument,

From 3fb7da56178e72887d570c2ecb99ee68f7511542 Mon Sep 17 00:00:00 2001
From: Jan Barta <55042ba...@sstebrno.eu>
Date: Fri, 3 Jun 2016 13:35:46 +0200
Subject: [PATCH 07/11] pylint: fix bad-mcs-classmethod-argument

---
 ipapython/install/util.py | 4 ++--
 pylintrc                  | 1 -
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/ipapython/install/util.py b/ipapython/install/util.py
index 1c264a8..37f1bfe 100644
--- a/ipapython/install/util.py
+++ b/ipapython/install/util.py
@@ -82,11 +82,11 @@ def run_generator_with_yield_from(gen):
 
 
 class InnerClassMeta(type):
-    def __new__(cls, name, bases, class_dict):
+    def __new__(mcs, name, bases, class_dict):
         class_dict.pop('__outer_class__', None)
         class_dict.pop('__outer_name__', None)
 
-        return super(InnerClassMeta, cls).__new__(cls, name, bases, class_dict)
+        return super(InnerClassMeta, mcs).__new__(mcs, name, bases, class_dict)
 
     def __get__(self, obj, obj_type):
         outer_class, outer_name = self.__bind(obj_type)
diff --git a/pylintrc b/pylintrc
index 115b295..f76aaf7 100644
--- a/pylintrc
+++ b/pylintrc
@@ -60,7 +60,6 @@ disable=
     unused-variable,
     useless-else-on-loop,
     bad-continuation,
-    bad-mcs-classmethod-argument,
     bad-mcs-method-argument,
     bad-whitespace,
     blacklisted-name,

From 1b844189955e191463230d00451bb8e84cde4902 Mon Sep 17 00:00:00 2001
From: Jan Barta <55042ba...@sstebrno.eu>
Date: Fri, 3 Jun 2016 13:43:21 +0200
Subject: [PATCH 08/11] pylint: fix bad-mcs-method-argument

---
 ipapython/install/util.py | 48 +++++++++++++++++++++++------------------------
 pylintrc                  |  1 -
 2 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/ipapython/install/util.py b/ipapython/install/util.py
index 37f1bfe..4c6bcd5 100644
--- a/ipapython/install/util.py
+++ b/ipapython/install/util.py
@@ -88,16 +88,16 @@ def __new__(mcs, name, bases, class_dict):
 
         return super(InnerClassMeta, mcs).__new__(mcs, name, bases, class_dict)
 
-    def __get__(self, obj, obj_type):
-        outer_class, outer_name = self.__bind(obj_type)
+    def __get__(cls, obj, obj_type):
+        outer_class, outer_name = cls.__bind(obj_type)
         if obj is None:
-            return self
+            return cls
         assert isinstance(obj, outer_class)
 
         try:
             return obj.__dict__[outer_name]
         except KeyError:
-            inner = self(obj)
+            inner = cls(obj)
             try:
                 getter = inner.__get__
             except AttributeError:
@@ -105,11 +105,11 @@ def __get__(self, obj, obj_type):
             else:
                 return getter(obj, obj_type)
 
-    def __set__(self, obj, value):
-        outer_class, outer_name = self.__bind(obj.__class__)
+    def __set__(cls, obj, value):
+        outer_class, outer_name = cls.__bind(obj.__class__)
         assert isinstance(obj, outer_class)
 
-        inner = self(obj)
+        inner = cls(obj)
         try:
             setter = inner.__set__
         except AttributeError:
@@ -122,11 +122,11 @@ def __set__(self, obj, value):
         else:
             setter(obj, value)
 
-    def __delete__(self, obj):
-        outer_class, outer_name = self.__bind(obj.__class__)
+    def __delete__(cls, obj):
+        outer_class, outer_name = cls.__bind(obj.__class__)
         assert isinstance(obj, outer_class)
 
-        inner = self(obj)
+        inner = cls(obj)
         try:
             deleter = inner.__delete__
         except AttributeError:
@@ -142,23 +142,23 @@ def __delete__(self, obj):
         else:
             deleter(obj)
 
-    def __bind(self, obj_type):
+    def __bind(cls, obj_type):
         try:
-            cls = self.__dict__['__outer_class__']
-            name = self.__dict__['__outer_name__']
+            outer_class = cls.__dict__['__outer_class__']
+            name = cls.__dict__['__outer_name__']
         except KeyError:
-            cls, name, value = None, None, None
-            for cls in obj_type.__mro__:
-                for name, value in six.iteritems(cls.__dict__):
-                    if value is self:
+            outer_class, name, value = None, None, None
+            for outer_class in obj_type.__mro__:
+                for name, value in six.iteritems(outer_class.__dict__):
+                    if value is cls:
                         break
-                if value is self:
+                if value is cls:
                     break
-            assert value is self
+            assert value is cls
 
-            self.__outer_class__ = cls
-            self.__outer_name__ = name
-            self.__name__ = '.'.join((cls.__name__, name))
-            self.__qualname__ = self.__name__
+            cls.__outer_class__ = outer_class
+            cls.__outer_name__ = name
+            cls.__name__ = '.'.join((outer_class.__name__, name))
+            cls.__qualname__ = cls.__name__
 
-        return cls, name
+        return outer_class, name
diff --git a/pylintrc b/pylintrc
index f76aaf7..70cf124 100644
--- a/pylintrc
+++ b/pylintrc
@@ -60,7 +60,6 @@ disable=
     unused-variable,
     useless-else-on-loop,
     bad-continuation,
-    bad-mcs-method-argument,
     bad-whitespace,
     blacklisted-name,
     invalid-name,

From b4af2497361c19862896b4efbf6aa0359ff898f4 Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Thu, 22 Sep 2016 13:46:22 +0200
Subject: [PATCH 09/11] Pylint: enable cyclic-import check

It looks that pylint stopped printing false positive errors for
cyclic-import check, thus check can be enabled.
---
 pylintrc | 1 -
 1 file changed, 1 deletion(-)

diff --git a/pylintrc b/pylintrc
index 70cf124..28857b8 100644
--- a/pylintrc
+++ b/pylintrc
@@ -17,7 +17,6 @@ enable=
 
 disable=
     I,
-    cyclic-import,
     duplicate-code,
     import-error,
     interface-not-implemented,

From 1a0537f131a2c89d55b43b52ec23f755ca8bc3dd Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Thu, 22 Sep 2016 14:12:14 +0200
Subject: [PATCH 10/11] Test: dont use global variable for iteration in
 test_cert_plugin

Iteration over global variable causes unwanted value changes outside
method

https://fedorahosted.org/freeipa/ticket/5755
---
 ipatests/test_xmlrpc/test_cert_plugin.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/ipatests/test_xmlrpc/test_cert_plugin.py b/ipatests/test_xmlrpc/test_cert_plugin.py
index fb4ab58..2598e0b 100644
--- a/ipatests/test_xmlrpc/test_cert_plugin.py
+++ b/ipatests/test_xmlrpc/test_cert_plugin.py
@@ -215,7 +215,9 @@ def test_0007_service_show(self):
         res = api.Command['service_show'](self.service_princ)['result']
 
         # Both the old and the new certs should be listed as certificates now
-        certs_encoded = (base64.b64encode(cert) for cert in res['usercertificate'])
+        certs_encoded = (
+            base64.b64encode(usercert) for usercert in res['usercertificate']
+        )
         assert set(certs_encoded) == set([cert, newcert])
 
     def test_0008_cleanup(self):

From 6f426e8fbc6ef94c4098b4d6080a74be510710e1 Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Thu, 22 Sep 2016 14:39:16 +0200
Subject: [PATCH 11/11] Pylint: enable global-variable-not-assigned check

the global keyword should be used only when variable from outside is
assigned inside, otherwise it has no effect and just confuses developers
---
 daemons/dnssec/ipa-dnskeysyncd           | 4 +++-
 install/tools/ipa-replica-conncheck      | 1 -
 ipaserver/install/dns.py                 | 3 ---
 ipaserver/install/sysupgrade.py          | 3 ---
 ipatests/test_xmlrpc/test_cert_plugin.py | 6 ------
 pylintrc                                 | 1 -
 6 files changed, 3 insertions(+), 15 deletions(-)

diff --git a/daemons/dnssec/ipa-dnskeysyncd b/daemons/dnssec/ipa-dnskeysyncd
index a381c29..dfe4cb4 100755
--- a/daemons/dnssec/ipa-dnskeysyncd
+++ b/daemons/dnssec/ipa-dnskeysyncd
@@ -38,7 +38,9 @@ KEYTAB_FB = paths.IPA_DNSKEYSYNCD_KEYTAB
 # Shutdown handler
 def commenceShutdown(signum, stack):
     # Declare the needed global variables
-    global watcher_running, ldap_connection, log
+    global watcher_running
+    global ldap_connection  # pylint: disable=global-variable-not-assigned
+
     log.info('Signal %s received: Shutting down!', signum)
 
     # We are no longer running
diff --git a/install/tools/ipa-replica-conncheck b/install/tools/ipa-replica-conncheck
index a192bbf..0c583cf 100755
--- a/install/tools/ipa-replica-conncheck
+++ b/install/tools/ipa-replica-conncheck
@@ -385,7 +385,6 @@ def main():
         print_info("\nConnection from replica to master is OK.")
 
         # create listeners
-        global RESPONDERS
         print_info("Start listening on required ports for remote master check")
 
         for port in required_ports:
diff --git a/ipaserver/install/dns.py b/ipaserver/install/dns.py
index dedafec..912b94f 100644
--- a/ipaserver/install/dns.py
+++ b/ipaserver/install/dns.py
@@ -314,9 +314,6 @@ def install_check(standalone, api, replica, options, hostname):
 
 
 def install(standalone, replica, options, api=api):
-    global ip_addresses
-    global reverse_zones
-
     local_dnskeysyncd_dn = DN(('cn', 'DNSKeySync'), ('cn', api.env.host),
                               ('cn', 'masters'), ('cn', 'ipa'), ('cn', 'etc'),
                               api.env.basedn)
diff --git a/ipaserver/install/sysupgrade.py b/ipaserver/install/sysupgrade.py
index 1eba38c..e9192ac 100644
--- a/ipaserver/install/sysupgrade.py
+++ b/ipaserver/install/sysupgrade.py
@@ -35,17 +35,14 @@ def _load_sstore():
 
 def get_upgrade_state(module, state):
     _load_sstore()
-    global _sstore
     return _sstore.get_state(module, state)
 
 def set_upgrade_state(module, state, value):
     _load_sstore()
-    global _sstore
     _sstore.backup_state(module, state, value)
 
 def remove_upgrade_state(module, state):
     _load_sstore()
-    global _sstore
     _sstore.delete_state(module, state)
 
 def remove_upgrade_file():
diff --git a/ipatests/test_xmlrpc/test_cert_plugin.py b/ipatests/test_xmlrpc/test_cert_plugin.py
index 2598e0b..ab09d0a 100644
--- a/ipatests/test_xmlrpc/test_cert_plugin.py
+++ b/ipatests/test_xmlrpc/test_cert_plugin.py
@@ -166,8 +166,6 @@ def test_0003_service_show(self):
         """
         Verify that service-show has the right certificate using service-show.
         """
-        global cert
-
         res = api.Command['service_show'](self.service_princ)['result']
         assert base64.b64encode(res['usercertificate'][0]) == cert
 
@@ -175,8 +173,6 @@ def test_0004_service_find(self):
         """
         Verify that service-find has the right certificate using service-find.
         """
-        global cert
-
         # Assume there is only one service
         res = api.Command['service_find'](self.service_princ)['result']
         assert base64.b64encode(res[0]['usercertificate'][0]) == cert
@@ -210,8 +206,6 @@ def test_0007_service_show(self):
         """
         Verify the new certificate with service-show.
         """
-        global cert, newcert
-
         res = api.Command['service_show'](self.service_princ)['result']
 
         # Both the old and the new certs should be listed as certificates now
diff --git a/pylintrc b/pylintrc
index 28857b8..5f867bd 100644
--- a/pylintrc
+++ b/pylintrc
@@ -46,7 +46,6 @@ disable=
     exec-used,
     fixme,
     global-statement,
-    global-variable-not-assigned,
     no-init,
     pointless-string-statement,
     protected-access,
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to