URL: https://github.com/freeipa/freeipa/pull/2152 Author: tiran Title: #2152: Teach pylint how our api works Action: opened
PR body: """ pylint 2.0 is more strict and complains about several aspects of ipalib.api. It turns out that AstroidBuilder.string_build() can be used to easily teach pylint about object attributes and attribute values. Although the assignment wouldn't work with the actual implementation, the string builder assignments shows pylint the names and values of members. It works without additional transformation. See: pagure.io/freeipa/issue/7614 Signed-off-by: Christian Heimes <chei...@redhat.com> """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/2152/head:pr2152 git checkout pr2152
From 3e844e4ee85440a292ab16f941c24320c0702ba5 Mon Sep 17 00:00:00 2001 From: Christian Heimes <chei...@redhat.com> Date: Fri, 13 Jul 2018 10:15:29 +0200 Subject: [PATCH 1/2] Teach pylint how our api works pylint 2.0 is more strict and complains about several aspects of ipalib.api. It turns out that AstroidBuilder.string_build() can be used to easily teach pylint about object attributes and attribute values. Although the assignment wouldn't work with the actual implementation, the string builder assignments shows pylint the names and values of members. It works without additional transformation. See: https://pagure.io/freeipa/issue/7614 Signed-off-by: Christian Heimes <chei...@redhat.com> --- pylint_plugins.py | 216 ++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 168 insertions(+), 48 deletions(-) diff --git a/pylint_plugins.py b/pylint_plugins.py index 896f954abe..ee00d1612f 100644 --- a/pylint_plugins.py +++ b/pylint_plugins.py @@ -58,30 +58,6 @@ def fake_class(name_or_class_obj, members=()): return cl -fake_backend = {'Backend': [ - {'wsgi_dispatch': ['mount']}, -]} - -NAMESPACE_ATTRS = ['Object', 'Method', fake_backend, 'Updater', - 'Advice'] -fake_api_env = { - 'env': [ - 'host', - 'realm', - 'kinit_lifetime', - ], - 'Command': [ - 'get_plugin', - 'config_show', - 'host_show', - 'service_show', - 'user_show', - ], -} - -# this is due ipaserver.rpcserver.KerberosSession where api is undefined -fake_api = {'api': [fake_api_env] + NAMESPACE_ATTRS} - # 'class': ['generated', 'properties'] ipa_class_members = { # Python standard library & 3rd party classes @@ -96,21 +72,6 @@ def fake_class(name_or_class_obj, members=()): 'find' ], 'ipalib.cli.Collector': ['__options'], - 'ipalib.config.Env': [ - {'__d': ['get']}, - {'__done': ['add']}, - 'ca_host', - 'ca_install_port', - 'debug', - {'domain': dir(str)}, - 'http_timeout', - 'rpc_protocol', - 'startup_traceback', - 'server', - 'validate_api', - 'verbose', - 'xmlrpc_uri', - ], 'ipalib.errors.ACIError': [ 'info', ], @@ -205,12 +166,6 @@ def fake_class(name_or_class_obj, members=()): 'require_service', ], 'ipalib.plugable.API': [ - fake_api_env, - ] + NAMESPACE_ATTRS, - 'ipalib.plugable.Plugin': [ - 'Object', - 'Method', - 'Updater', 'Advice', ], 'ipalib.util.ForwarderValidationError': [ @@ -220,9 +175,6 @@ def fake_class(name_or_class_obj, members=()): 'validatedns', 'normalizedns', ], - 'ipaserver.rpcserver.KerberosSession': [ - fake_api, - ], 'ipatests.test_integration.base.IntegrationTest': [ 'domain', {'master': [ @@ -404,3 +356,171 @@ def visit_import(self, node): def visit_importfrom(self, node): names = ['{}.{}'.format(node.modname, n[0]) for n in node.names] self._check_forbidden_imports(node, names) + + +# +# Teach pylint how api object works +# +# ipalib uses some tricks to create api.env members and api objects. pylint +# is not able to infer member names and types from code. The explict +# assignments inside the string builder templates are good enough to show +# pylint, how the api is created. Additional transformations are not +# required. +# + +AstroidBuilder(MANAGER).string_build(textwrap.dedent( + """ + from ipalib import api + from ipalib import cli, plugable, rpc + from ipalib.base import NameSpace + from ipaclient.plugins import rpcclient + try: + from ipaserver.plugins import dogtag, ldap2, serverroles + except ImportError: + HAS_SERVER = False + else: + HAS_SERVER = True + + def wildcard(*args, **kwargs): + return None + + # ipalib.api members + api.Backend = plugable.APINameSpace(api, None) + api.Command = plugable.APINameSpace(api, None) + api.Method = plugable.APINameSpace(api, None) + api.Object = plugable.APINameSpace(api, None) + api.Updater = plugable.APINameSpace(api, None) + # ipalib.api.Backend members + api.Backend.cli = cli.cli(api) + api.Backend.textui = cli.textui(api) + api.Backend.jsonclient = rpc.jsonclient(api) + api.Backend.rpcclient = rpcclient.rpcclient(api) + api.Backend.xmlclient = rpc.xmlclient(api) + + if HAS_SERVER: + api.Backend.kra = dogtag.kra(api) + api.Backend.ldap2 = ldap2.ldap2(api) + api.Backend.ra = dogtag.ra(api) + api.Backend.ra_certprofile = dogtag.ra_certprofile(api) + api.Backend.ra_lightweight_ca = dogtag.ra_lightweight_ca(api) + api.Backend.serverroles = serverroles.serverroles(api) + + # ipalib.base.NameSpace + NameSpace.find = wildcard + """ +)) + + +AstroidBuilder(MANAGER).string_build(textwrap.dedent( + """ + from ipalib import api + from ipapython.dn import DN + + api.env.api_version = '' + api.env.bin = '' # object + api.env.ca_agent_install_port = None + api.env.ca_agent_port = 0 + api.env.ca_ee_install_port = None + api.env.ca_ee_port = 0 + api.env.ca_host = '' + api.env.ca_install_port = None + api.env.ca_port = 0 + api.env.conf = '' # object + api.env.conf_default = '' # object + api.env.confdir = '' # object + api.env.container_accounts = DN() + api.env.container_adtrusts = DN() + api.env.container_applications = DN() + api.env.container_automember = DN() + api.env.container_automount = DN() + api.env.container_ca = DN() + api.env.container_caacl = DN() + api.env.container_certmap = DN() + api.env.container_certmaprules = DN() + api.env.container_certprofile = DN() + api.env.container_cifsdomains = DN() + api.env.container_configs = DN() + api.env.container_custodia = DN() + api.env.container_deleteuser = DN() + api.env.container_dna = DN() + api.env.container_dna_posix_ids = DN() + api.env.container_dns = DN() + api.env.container_dnsservers = DN() + api.env.container_group = DN() + api.env.container_hbac = DN() + api.env.container_hbacservice = DN() + api.env.container_hbacservicegroup = DN() + api.env.container_host = DN() + api.env.container_hostgroup = DN() + api.env.container_locations = DN() + api.env.container_masters = DN() + api.env.container_netgroup = DN() + api.env.container_otp = DN() + api.env.container_permission = DN() + api.env.container_policies = DN() + api.env.container_policygroups = DN() + api.env.container_policylinks = DN() + api.env.container_privilege = DN() + api.env.container_radiusproxy = DN() + api.env.container_ranges = DN() + api.env.container_realm_domains = DN() + api.env.container_rolegroup = DN() + api.env.container_roles = DN() + api.env.container_s4u2proxy = DN() + api.env.container_selinux = DN() + api.env.container_service = DN() + api.env.container_stageuser = DN() + api.env.container_sudocmd = DN() + api.env.container_sudocmdgroup = DN() + api.env.container_sudorule = DN() + api.env.container_sysaccounts = DN() + api.env.container_topology = DN() + api.env.container_trusts = DN() + api.env.container_user = DN() + api.env.container_vault = DN() + api.env.container_views = DN() + api.env.container_virtual = DN() + api.env.context = '' # object + api.env.debug = False + api.env.delegate = False + api.env.dogtag_version = 0 + api.env.dot_ipa = '' # object + api.env.enable_ra = False + api.env.env_confdir = None + api.env.fallback = True + api.env.force_schema_check = False + api.env.home = '' # object + api.env.host = '' + api.env.http_timeout = 0 + api.env.in_server = False # object + api.env.in_tree = False # object + api.env.interactive = True + api.env.ipalib = '' # object + api.env.kinit_lifetime = None + api.env.log = '' # object + api.env.logdir = '' # object + api.env.mode = '' + api.env.mount_ipa = '' + api.env.nss_dir = '' # object + api.env.plugins_on_demand = False # object + api.env.prompt_all = False + api.env.ra_plugin = '' + api.env.recommended_max_agmts = 0 + api.env.replication_wait_timeout = 0 + api.env.rpc_protocol = '' + api.env.server = '' + api.env.script = '' # object + api.env.site_packages = '' # object + api.env.skip_version_check = False + api.env.startup_timeout = 0 + api.env.startup_traceback = False + api.env.tls_ca_cert = '' # object + api.env.tls_version_max = '' + api.env.tls_version_min = '' + api.env.validate_api = False + api.env.verbose = 0 + api.env.version = '' + api.env.wait_for_dns = 0 + api.env.webui_prod = True + """ +)) From fde6a713fed2329b03bca6886e85a03af19c5b69 Mon Sep 17 00:00:00 2001 From: Christian Heimes <chei...@redhat.com> Date: Fri, 13 Jul 2018 11:07:06 +0200 Subject: [PATCH 2/2] Add pylint ignore to magic config.Env attributes pylinti 2 is having a hard time to handle name mangled, magic attributes correctly. Double under attributes like __d are internally renamed to _Env__d. After multiple failed attempts, it was easier to just add more pylint disable to the implementation. pylint 2 also thinkgs that Env.server is defined much later or the env doesn't have that member at all. Ignore the false warnings, too. Signed-off-by: Christian Heimes <chei...@redhat.com> --- ipalib/config.py | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/ipalib/config.py b/ipalib/config.py index 52b032a25a..f354547009 100644 --- a/ipalib/config.py +++ b/ipalib/config.py @@ -245,10 +245,12 @@ def __setitem__(self, key, value): SET_ERROR % (self.__class__.__name__, key, value) ) check_name(key) + # pylint: disable=no-member if key in self.__d: raise AttributeError(OVERRIDE_ERROR % (self.__class__.__name__, key, self.__d[key], value) ) + # pylint: enable=no-member assert not hasattr(self, key) if isinstance(value, six.string_types): value = value.strip() @@ -269,15 +271,15 @@ def __setitem__(self, key, value): if type(value) not in (unicode, int, float, bool, type(None), DN): raise TypeError(key, value) object.__setattr__(self, key, value) - # pylint: disable=unsupported-assignment-operation + # pylint: disable=unsupported-assignment-operation, no-member self.__d[key] = value - # pylint: enable=unsupported-assignment-operation + # pylint: enable=unsupported-assignment-operation, no-member def __getitem__(self, key): """ Return the value corresponding to ``key``. """ - return self.__d[key] + return self.__d[key] # pylint: disable=no-member def __delattr__(self, name): """ @@ -300,19 +302,19 @@ def __contains__(self, key): """ Return True if instance contains ``key``; otherwise return False. """ - return key in self.__d + return key in self.__d # pylint: disable=no-member def __len__(self): """ Return number of variables currently set. """ - return len(self.__d) + return len(self.__d) # pylint: disable=no-member def __iter__(self): """ Iterate through keys in ascending order. """ - for key in sorted(self.__d): + for key in sorted(self.__d): # pylint: disable=no-member yield key def _merge(self, **kw): @@ -405,6 +407,7 @@ def _join(self, key, *parts): return None def __doing(self, name): + # pylint: disable=no-member if name in self.__done: raise Exception( '%s.%s() already called' % (self.__class__.__name__, name) @@ -412,11 +415,11 @@ def __doing(self, name): self.__done.add(name) def __do_if_not_done(self, name): - if name not in self.__done: + if name not in self.__done: # pylint: disable=no-member getattr(self, name)() def _isdone(self, name): - return name in self.__done + return name in self.__done # pylint: disable=no-member def _bootstrap(self, **overrides): """ @@ -557,7 +560,8 @@ def _finalize_core(self, **defaults): self.__do_if_not_done('_bootstrap') # Merge in context config file and then default config file: - if self.__d.get('mode', None) != 'dummy': + mode = self.__d.get('mode') # pylint: disable=no-member + if mode != 'dummy': self._merge_from_file(self.conf) self._merge_from_file(self.conf_default) @@ -594,10 +598,12 @@ def _finalize_core(self, **defaults): # and server from jsonrpc_uri so that when only server or xmlrpc_uri # is specified, all 3 keys have a value.) if 'xmlrpc_uri' not in self and 'server' in self: + # pylint: disable=no-member, access-member-before-definition self.xmlrpc_uri = 'https://{}/ipa/xml'.format(self.server) # Derive ldap_uri from server if 'ldap_uri' not in self and 'server' in self: + # pylint: disable=no-member, access-member-before-definition self.ldap_uri = 'ldap://{}'.format(self.server) # Derive jsonrpc_uri from xmlrpc_uri
_______________________________________________ FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/freeipa-devel@lists.fedorahosted.org/message/XUGZWUZVCLE45FL2JZICTHOQIYMVXF2T/
