From d8aa3e10398d0d23eefdbda899475ca32ec3abf6 Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Thu, 7 May 2015 12:33:40 -0400 Subject: pylint 1.4.3 version fixes 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 calsses. 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 funciotning properly. Signed-off-by: Simo Sorce --- Makefile | 2 ++ ipsilon/admin/common.py | 10 ++++------ ipsilon/admin/loginstack.py | 1 - ipsilon/info/infoldap.py | 2 +- ipsilon/info/infosssd.py | 2 +- ipsilon/login/authfas.py | 1 - ipsilon/login/authform.py | 2 +- ipsilon/login/authgssapi.py | 2 +- ipsilon/login/authldap.py | 1 - ipsilon/login/authpam.py | 1 - ipsilon/login/authtest.py | 1 - ipsilon/login/common.py | 1 - ipsilon/providers/openid/auth.py | 3 +-- ipsilon/providers/saml2/admin.py | 14 +++++++------- ipsilon/providers/saml2/auth.py | 1 - ipsilon/providers/saml2/provider.py | 2 +- ipsilon/tools/files.py | 2 +- ipsilon/util/config.py | 18 +++++++++--------- ipsilon/util/data.py | 1 - ipsilon/util/endpoint.py | 1 - ipsilon/util/errors.py | 1 - ipsilon/util/page.py | 2 -- ipsilon/util/user.py | 2 +- tests/helpers/http.py | 1 - 24 files changed, 30 insertions(+), 44 deletions(-) diff --git a/Makefile b/Makefile index df6eb0f..c02b6ae 100644 --- 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 diff --git a/ipsilon/admin/common.py b/ipsilon/admin/common.py index 87bfcd5..64334c2 100644 --- a/ipsilon/admin/common.py +++ b/ipsilon/admin/common.py @@ -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 diff --git a/ipsilon/admin/loginstack.py b/ipsilon/admin/loginstack.py index 5fdda96..1da1eae 100644 --- a/ipsilon/admin/loginstack.py +++ b/ipsilon/admin/loginstack.py @@ -59,5 +59,4 @@ class LoginStack(AdminPlugins): kwargs['sections'].append(targs) - # pylint: disable=star-args return self._template(self.template, **kwargs) diff --git a/ipsilon/info/infoldap.py b/ipsilon/info/infoldap.py index 84b4098..f87b37c 100644 --- a/ipsilon/info/infoldap.py +++ b/ipsilon/info/infoldap.py @@ -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 diff --git a/ipsilon/info/infosssd.py b/ipsilon/info/infosssd.py index 69d68c0..ee5f387 100644 --- a/ipsilon/info/infosssd.py +++ b/ipsilon/info/infosssd.py @@ -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) diff --git a/ipsilon/login/authfas.py b/ipsilon/login/authfas.py index 1489f73..996855c 100644 --- a/ipsilon/login/authfas.py +++ b/ipsilon/login/authfas.py @@ -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): diff --git a/ipsilon/login/authform.py b/ipsilon/login/authform.py index ecce919..0e20a60 100644 --- a/ipsilon/login/authform.py +++ b/ipsilon/login/authform.py @@ -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) diff --git a/ipsilon/login/authgssapi.py b/ipsilon/login/authgssapi.py index 3ef7616..1fac5ed 100644 --- a/ipsilon/login/authgssapi.py +++ b/ipsilon/login/authgssapi.py @@ -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) diff --git a/ipsilon/login/authldap.py b/ipsilon/login/authldap.py index 595d6be..2882897 100644 --- a/ipsilon/login/authldap.py +++ b/ipsilon/login/authldap.py @@ -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) diff --git a/ipsilon/login/authpam.py b/ipsilon/login/authpam.py index ba8ecdd..d703aa2 100644 --- a/ipsilon/login/authpam.py +++ b/ipsilon/login/authpam.py @@ -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) diff --git a/ipsilon/login/authtest.py b/ipsilon/login/authtest.py index 7769650..002ab73 100644 --- a/ipsilon/login/authtest.py +++ b/ipsilon/login/authtest.py @@ -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) diff --git a/ipsilon/login/common.py b/ipsilon/login/common.py index 6e21635..c7c8050 100644 --- a/ipsilon/login/common.py +++ b/ipsilon/login/common.py @@ -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): diff --git a/ipsilon/providers/openid/auth.py b/ipsilon/providers/openid/auth.py index 2510ff4..e85890e 100644 --- a/ipsilon/providers/openid/auth.py +++ b/ipsilon/providers/openid/auth.py @@ -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): diff --git a/ipsilon/providers/saml2/admin.py b/ipsilon/providers/saml2/admin.py index 158e590..5ab8f7e 100644 --- a/ipsilon/providers/saml2/admin.py +++ b/ipsilon/providers/saml2/admin.py @@ -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 diff --git a/ipsilon/providers/saml2/auth.py b/ipsilon/providers/saml2/auth.py index 8b84bc2..9d2bb7d 100644 --- a/ipsilon/providers/saml2/auth.py +++ b/ipsilon/providers/saml2/auth.py @@ -316,7 +316,6 @@ class AuthenticateRequest(ProviderPageBase): ], "submit": 'Return to application', } - # pylint: disable=star-args return self._template('saml2/post_response.html', **context) else: diff --git a/ipsilon/providers/saml2/provider.py b/ipsilon/providers/saml2/provider.py index 5d36fbd..75bfc1d 100644 --- a/ipsilon/providers/saml2/provider.py +++ b/ipsilon/providers/saml2/provider.py @@ -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) diff --git a/ipsilon/tools/files.py b/ipsilon/tools/files.py index 5847654..857a2fe 100644 --- a/ipsilon/tools/files.py +++ b/ipsilon/tools/files.py @@ -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) diff --git a/ipsilon/util/config.py b/ipsilon/util/config.py index 5366a96..a20c87c 100644 --- a/ipsilon/util/config.py +++ b/ipsilon/util/config.py @@ -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: diff --git a/ipsilon/util/data.py b/ipsilon/util/data.py index 0d1c2df..eec00b5 100644 --- a/ipsilon/util/data.py +++ b/ipsilon/util/data.py @@ -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 diff --git a/ipsilon/util/endpoint.py b/ipsilon/util/endpoint.py index 20d3694..92dc388 100644 --- a/ipsilon/util/endpoint.py +++ b/ipsilon/util/endpoint.py @@ -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() diff --git a/ipsilon/util/errors.py b/ipsilon/util/errors.py index 7017a1b..70d2da9 100644 --- a/ipsilon/util/errors.py +++ b/ipsilon/util/errors.py @@ -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 diff --git a/ipsilon/util/page.py b/ipsilon/util/page.py index ec3828d..094a6a9 100644 --- a/ipsilon/util/page.py +++ b/ipsilon/util/page.py @@ -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) diff --git a/ipsilon/util/user.py b/ipsilon/util/user.py index 38449cc..dd4a0f4 100644 --- a/ipsilon/util/user.py +++ b/ipsilon/util/user.py @@ -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), diff --git a/tests/helpers/http.py b/tests/helpers/http.py index 97098c8..bfa3240 100755 --- a/tests/helpers/http.py +++ b/tests/helpers/http.py @@ -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: -- cgit