pylint 1.4.3 version fixes
authorSimo Sorce <simo@redhat.com>
Thu, 7 May 2015 16:33:40 +0000 (12:33 -0400)
committerRob Crittenden <rcritten@redhat.com>
Thu, 7 May 2015 18:44:20 +0000 (14:44 -0400)
Pylint 1.4.3 completely stopped recognizing the star-args condition.
In order to avoid pylint error with > 1.4.3 stop caring for star-args
and add cmdline option to ignore those errors completly so older pylint
versions are happy too.

Also fix type() vs isinstance() checks, isinstance is generally a more
correct approach to check for classes.

In some 'admin' files the type() -> isinstance() fix required to invert
the order in which ComplexList and MappingList are checked as the latter
is a subclass of ComplexList, so it needs to be checked first otherwise
the check for isinstance(option, ComplexList) matches for both and the
code stops functioning properly.

Signed-off-by: Simo Sorce <simo@redhat.com>
Reviewed-by: Rob Crittenden <rcritten@redhat.com>
24 files changed:
Makefile
ipsilon/admin/common.py
ipsilon/admin/loginstack.py
ipsilon/info/infoldap.py
ipsilon/info/infosssd.py
ipsilon/login/authfas.py
ipsilon/login/authform.py
ipsilon/login/authgssapi.py
ipsilon/login/authldap.py
ipsilon/login/authpam.py
ipsilon/login/authtest.py
ipsilon/login/common.py
ipsilon/providers/openid/auth.py
ipsilon/providers/saml2/admin.py
ipsilon/providers/saml2/auth.py
ipsilon/providers/saml2/provider.py
ipsilon/tools/files.py
ipsilon/util/config.py
ipsilon/util/data.py
ipsilon/util/endpoint.py
ipsilon/util/errors.py
ipsilon/util/page.py
ipsilon/util/user.py
tests/helpers/http.py

index df6eb0f..c02b6ae 100644 (file)
--- a/Makefile
+++ b/Makefile
@@ -42,6 +42,7 @@ lint:
        pylint -d c,r,i,W0613 -r n -f colorized \
                   --notes= \
                   --ignored-classes=cherrypy,API \
+                  --disable=star-args \
                   ./ipsilon
 
 pep8:
@@ -77,6 +78,7 @@ lp-test:
        pylint -d c,r,i,W0613 -r n -f colorized \
                   --notes= \
                   --ignored-classes=cherrypy \
+                  --disable=star-args \
                   ./tests
        pep8 tests
 
index 87bfcd5..64334c2 100644 (file)
@@ -97,14 +97,14 @@ class AdminPluginConfig(AdminPage):
                         aname = '%s_%s' % (name, a)
                         if aname in kwargs:
                             value.append(a)
-                elif type(option) is pconfig.ComplexList:
-                    value = get_complex_list_value(name,
+                elif isinstance(option, pconfig.MappingList):
+                    value = get_mapping_list_value(name,
                                                    option.get_value(),
                                                    **kwargs)
                     if value is None:
                         continue
-                elif type(option) is pconfig.MappingList:
-                    value = get_mapping_list_value(name,
+                elif isinstance(option, pconfig.ComplexList):
+                    value = get_complex_list_value(name,
                                                    option.get_value(),
                                                    **kwargs)
                     if value is None:
@@ -257,7 +257,6 @@ class AdminPlugins(AdminPage):
             targs['order_name'] = '%s_order_form' % self.name
             targs['order_action'] = self.order.url
 
-        # pylint: disable=star-args
         return self._template(self.template, **targs)
 
     def root(self, *args, **kwargs):
@@ -351,7 +350,6 @@ class Admin(AdminPage):
     def scheme(self):
         cherrypy.response.headers.update({'Content-Type': 'image/svg+xml'})
         urls = self.get_menu_urls()
-        # pylint: disable=star-args
         return str(self._template('admin/ipsilon-scheme.svg', **urls))
     scheme.public_function = True
 
index 5fdda96..1da1eae 100644 (file)
@@ -59,5 +59,4 @@ class LoginStack(AdminPlugins):
 
             kwargs['sections'].append(targs)
 
-        # pylint: disable=star-args
         return self._template(self.template, **kwargs)
index 84b4098..f87b37c 100644 (file)
@@ -119,7 +119,7 @@ Info plugin that uses LDAP to retrieve user data. """
             raise Exception('No unique user object could be found!')
         data = dict()
         for name, value in result[0][1].iteritems():
-            if type(value) is list and len(value) == 1:
+            if isinstance(value, list) and len(value) == 1:
                 value = value[0]
             data[name] = value
         return data
index 69d68c0..ee5f387 100644 (file)
@@ -142,7 +142,7 @@ class Installer(InfoProviderInstaller):
         confopts = {'instance': opts['instance']}
 
         tmpl = Template(CONF_TEMPLATE)
-        hunk = tmpl.substitute(**confopts)  # pylint: disable=star-args
+        hunk = tmpl.substitute(**confopts)
         with open(opts['httpd_conf'], 'a') as httpd_conf:
             httpd_conf.write(hunk)
 
index 1489f73..996855c 100644 (file)
@@ -80,7 +80,6 @@ class FAS(LoginFormBase):
             error_username=not username
         )
         self.lm.set_auth_error()
-        # pylint: disable=star-args
         return self._template(self.formtemplate, **context)
 
     def make_userdata(self, fas_data):
index ecce919..0e20a60 100644 (file)
@@ -123,7 +123,7 @@ class Installer(LoginManagerInstaller):
                     'service': opts['form_service']}
 
         tmpl = Template(CONF_TEMPLATE)
-        hunk = tmpl.substitute(**confopts)  # pylint: disable=star-args
+        hunk = tmpl.substitute(**confopts)
         with open(opts['httpd_conf'], 'a') as httpd_conf:
             httpd_conf.write(hunk)
 
index 3ef7616..1fac5ed 100644 (file)
@@ -147,7 +147,7 @@ class Installer(LoginManagerInstaller):
             confopts['gssapisslonly'] = 'On'
 
         tmpl = Template(CONF_TEMPLATE)
-        hunk = tmpl.substitute(**confopts)  # pylint: disable=star-args
+        hunk = tmpl.substitute(**confopts)
         with open(opts['httpd_conf'], 'a') as httpd_conf:
             httpd_conf.write(hunk)
 
index 595d6be..2882897 100644 (file)
@@ -86,7 +86,6 @@ class LDAP(LoginFormBase, Log):
             error_username=not username
         )
         self.lm.set_auth_error()
-        # pylint: disable=star-args
         return self._template('login/form.html', **context)
 
 
index ba8ecdd..d703aa2 100644 (file)
@@ -62,7 +62,6 @@ class Pam(LoginFormBase):
             error_username=not username
         )
         self.lm.set_auth_error()
-        # pylint: disable=star-args
         return self._template('login/form.html', **context)
 
 
index 7769650..002ab73 100644 (file)
@@ -56,7 +56,6 @@ class TestAuth(LoginFormBase):
             error_username=not username
         )
         self.lm.set_auth_error()
-        # pylint: disable=star-args
         return self._template('login/form.html', **context)
 
 
index 6e21635..c7c8050 100644 (file)
@@ -185,7 +185,6 @@ class LoginFormBase(LoginPageBase):
 
     def GET(self, *args, **kwargs):
         context = self.create_tmpl_context()
-        # pylint: disable=star-args
         return self._template(self.formtemplate, **context)
 
     def root(self, *args, **kwargs):
index 2510ff4..e85890e 100644 (file)
@@ -44,7 +44,7 @@ class AuthenticateRequest(ProviderPageBase):
         if args is not None:
             first = args[0] if len(args) > 0 else None
             second = first[0] if len(first) > 0 else None
-            if type(second) is dict:
+            if isinstance(second, dict):
                 form = second.get('form', None)
         return form
 
@@ -191,7 +191,6 @@ class AuthenticateRequest(ProviderPageBase):
                 "authz_details": ad,
             }
             context.update(dict((self.trans.get_POST_tuple(),)))
-            # pylint: disable=star-args
             return self._template('openid/consent_form.html', **context)
 
     def _response(self, request, session):
index 158e590..5ab8f7e 100644 (file)
@@ -157,6 +157,9 @@ class SPAdminPage(AdminPage):
                 value = kwargs[name]
                 if isinstance(option, pconfig.List):
                     value = [x.strip() for x in value.split('\n')]
+                    # for normal lists we want unordered comparison
+                    if set(value) == set(option.get_value()):
+                        continue
                 elif isinstance(option, pconfig.Condition):
                     value = True
             else:
@@ -168,9 +171,9 @@ class SPAdminPage(AdminPage):
                         aname = '%s_%s' % (name, a)
                         if aname in kwargs:
                             value.append(a)
-                elif type(option) is pconfig.ComplexList:
+                elif isinstance(option, pconfig.MappingList):
                     current = deepcopy(option.get_value())
-                    value = get_complex_list_value(name,
+                    value = get_mapping_list_value(name,
                                                    current,
                                                    **kwargs)
                     # if current value is None do nothing
@@ -178,9 +181,9 @@ class SPAdminPage(AdminPage):
                         if option.get_value() is None:
                             continue
                         # else pass and let it continue as None
-                elif type(option) is pconfig.MappingList:
+                elif isinstance(option, pconfig.ComplexList):
                     current = deepcopy(option.get_value())
-                    value = get_mapping_list_value(name,
+                    value = get_complex_list_value(name,
                                                    current,
                                                    **kwargs)
                     # if current value is None do nothing
@@ -192,9 +195,6 @@ class SPAdminPage(AdminPage):
                     continue
 
             if value != option.get_value():
-                if (type(option) is pconfig.List and
-                        set(value) == set(option.get_value())):
-                    continue
                 cherrypy.log.error("Storing %s = %s" %
                                    (name, value), severity=logging.DEBUG)
                 new_db_values[name] = value
index 8b84bc2..9d2bb7d 100644 (file)
@@ -316,7 +316,6 @@ class AuthenticateRequest(ProviderPageBase):
                 ],
                 "submit": 'Return to application',
             }
-            # pylint: disable=star-args
             return self._template('saml2/post_response.html', **context)
 
         else:
index 5d36fbd..75bfc1d 100644 (file)
@@ -133,7 +133,7 @@ class ServiceProvider(ServiceProviderConfig):
 
     @allowed_nameids.setter
     def allowed_nameids(self, value):
-        if type(value) is not list:
+        if not isinstance(value, list):
             raise ValueError("Must be a list")
         self._staging['allowed nameids'] = ','.join(value)
 
index 5847654..857a2fe 100644 (file)
@@ -39,6 +39,6 @@ def fix_user_dirs(path, user=None, mode=0700):
 def write_from_template(destfile, template, opts):
     with open(template) as f:
         t = Template(f.read())
-    text = t.substitute(**opts)  # pylint: disable=star-args
+    text = t.substitute(**opts)
     with open(destfile, 'w+') as f:
         f.write(text)
index 5366a96..a20c87c 100644 (file)
@@ -109,7 +109,7 @@ class Option(Log):
         return None
 
     def _str_import_value(self, value):
-        if type(value) is not str:
+        if not isinstance(value, str):
             raise ValueError('Value must be string')
         self._assigned_value = value
 
@@ -170,7 +170,7 @@ class List(Option):
         return None
 
     def import_value(self, value):
-        if type(value) is not str:
+        if not isinstance(value, str):
             raise ValueError('Value (type: %s) must be string' % type(value))
         self._assigned_value = [x.strip() for x in value.split(',')]
 
@@ -180,7 +180,7 @@ class ComplexList(List):
     def _check_value(self, value):
         if value is None:
             return
-        if type(value) is not list:
+        if not isinstance(value, list):
             raise ValueError('The value type must be a list, not "%s"' %
                              type(value))
 
@@ -194,7 +194,7 @@ class ComplexList(List):
         return None
 
     def import_value(self, value):
-        if type(value) is not str:
+        if not isinstance(value, str):
             raise ValueError('The value type must be a string, not "%s"' %
                              type(value))
         jsonval = json.loads(value)
@@ -206,11 +206,11 @@ class MappingList(ComplexList):
     def _check_value(self, value):
         if value is None:
             return
-        if type(value) is not list:
+        if not isinstance(value, list):
             raise ValueError('The value type must be a list, not "%s"' %
                              type(value))
         for v in value:
-            if type(v) is not list:
+            if not isinstance(v, list):
                 raise ValueError('Each element must be a list, not "%s"' %
                                  type(v))
             if len(v) != 2:
@@ -218,7 +218,7 @@ class MappingList(ComplexList):
                                  ' not %d' % len(v))
 
     def import_value(self, value):
-        if type(value) is not str:
+        if not isinstance(value, str):
             raise ValueError('Value (type: %s) must be string' % type(value))
         jsonval = json.loads(value)
         self.set_value(jsonval)
@@ -253,7 +253,7 @@ class Choice(Option):
         return '%s=%s' % (self.name, self.get_value())
 
     def set_value(self, value):
-        if type(value) is not list:
+        if not isinstance(value, list):
             value = [value]
         self._assigned_value = list()
         for val in value:
@@ -267,7 +267,7 @@ class Choice(Option):
             self._assigned_value = None
 
     def unset_value(self, value):
-        if type(value) is str:
+        if isinstance(value, str):
             value = [value]
         unset = list()
         for val in value:
index 0d1c2df..eec00b5 100644 (file)
@@ -58,7 +58,6 @@ class SqlStore(Log):
             # It's not possible to share connections for SQLite between
             #  threads, so let's use the SingletonThreadPool for them
             pool_args = {'poolclass': SingletonThreadPool}
-        # pylint: disable=star-args
         self._dbengine = create_engine(engine_name, **pool_args)
         self.is_readonly = False
 
index 20d3694..92dc388 100644 (file)
@@ -62,7 +62,6 @@ class Endpoint(Log):
         return False
 
     def __call__(self, *args, **kwargs):
-        # pylint: disable=star-args
         cherrypy.response.headers.update(self.default_headers)
 
         self.user = UserSession().get_user()
index 7017a1b..70d2da9 100644 (file)
@@ -24,7 +24,6 @@ class Errors(Page):
         super(Errors, self).__init__(*args, **kwargs)
 
     def _error_template(self, *args, **kwargs):
-        # pylint: disable=star-args
         output_page = self._template(*args, **kwargs)
         # for some reason cherrypy will choke if the output
         # is a unicode object, so use str() here to please it
index ec3828d..094a6a9 100644 (file)
@@ -71,7 +71,6 @@ class Page(Endpoint):
         return False
 
     def __call__(self, *args, **kwargs):
-        # pylint: disable=star-args
         cherrypy.response.headers.update(self.default_headers)
 
         self.user = UserSession().get_user()
@@ -116,7 +115,6 @@ class Page(Endpoint):
         return model
 
     def _template(self, *args, **kwargs):
-        # pylint: disable=star-args
         t = self._site['template_env'].get_template(args[0])
         m = self._template_model()
         m.update(kwargs)
index 38449cc..dd4a0f4 100644 (file)
@@ -140,7 +140,7 @@ class UserSession(Log):
 
     def logout(self, user):
         if user is not None:
-            if not type(user) is User:
+            if not isinstance(user, User):
                 raise TypeError
             # Completely reset user data
             cherrypy.log.error('%s %s' % (user.name, user.fullname),
index 97098c8..bfa3240 100755 (executable)
@@ -265,7 +265,6 @@ class HttpSessions(object):
         args = {}
 
         while True:
-            # pylint: disable=star-args
             r = self.access(action, url, krb=krb, **args)
             if r.status_code == 303 or r.status_code == 302:
                 if not follow_redirect: