From 6bad4e57bd28d7433e5144b60e57c8cf042ebb3b Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Sun, 26 Mar 2017 09:24:30 -0700 Subject: [PATCH] Migrate most uses of if type() to if isinstance() Also convert those checks to use abcs instead of dict and list. Make a sentinel class for strategies to report when they've reache the end --- contrib/inventory/ec2.py | 4 ++-- contrib/inventory/packet_net.py | 2 +- contrib/inventory/ssh_config.py | 9 +++++--- lib/ansible/plugins/callback/dense.py | 9 +++++--- lib/ansible/plugins/callback/log_plays.py | 3 ++- lib/ansible/plugins/filter/core.py | 22 +++++++++---------- lib/ansible/plugins/lookup/csvfile.py | 3 ++- lib/ansible/plugins/lookup/dig.py | 5 +++-- lib/ansible/plugins/lookup/ini.py | 5 +++-- lib/ansible/plugins/strategy/__init__.py | 7 ++++-- lib/ansible/plugins/test/core.py | 11 +++++----- .../modules/cloud/openstack/test_os_server.py | 3 ++- 12 files changed, 49 insertions(+), 34 deletions(-) diff --git a/contrib/inventory/ec2.py b/contrib/inventory/ec2.py index 19fe52364c..7b3c6d44a8 100755 --- a/contrib/inventory/ec2.py +++ b/contrib/inventory/ec2.py @@ -1378,7 +1378,7 @@ class Ec2Inventory(object): elif key == 'ec2__previous_state': instance_vars['ec2_previous_state'] = instance.previous_state or '' instance_vars['ec2_previous_state_code'] = instance.previous_state_code - elif type(value) in [int, bool]: + elif isinstance(value, (int, bool)): instance_vars[key] = value elif isinstance(value, six.string_types): instance_vars[key] = value.strip() @@ -1483,7 +1483,7 @@ class Ec2Inventory(object): # Target: Everything # Preserve booleans and integers - elif type(value) in [int, bool]: + elif isinstance(value, (int, bool)): host_info[key] = value # Target: Everything diff --git a/contrib/inventory/packet_net.py b/contrib/inventory/packet_net.py index c40c821cb3..cd04cee111 100755 --- a/contrib/inventory/packet_net.py +++ b/contrib/inventory/packet_net.py @@ -385,7 +385,7 @@ class PacketInventory(object): device_vars[key] = device.state or '' elif key == 'packet_hostname': device_vars[key] = value - elif type(value) in [int, bool]: + elif isinstance(value, (int, bool)): device_vars[key] = value elif isinstance(value, six.string_types): device_vars[key] = value.strip() diff --git a/contrib/inventory/ssh_config.py b/contrib/inventory/ssh_config.py index ae41e5810f..dc8d96ea06 100755 --- a/contrib/inventory/ssh_config.py +++ b/contrib/inventory/ssh_config.py @@ -43,13 +43,16 @@ import argparse import os.path import sys -import paramiko +from collections import MutableSequence try: import json except ImportError: import simplejson as json +import paramiko + + SSH_CONF = '~/.ssh/config' _key = 'ssh_config' @@ -68,7 +71,7 @@ def get_config(): cfg.parse(f) ret_dict = {} for d in cfg._config: - if type(d['host']) is list: + if isinstance(d['host'], MutableSequence): alias = d['host'][0] else: alias = d['host'] @@ -93,7 +96,7 @@ def print_list(): # If the attribute is a list, just take the first element. # Private key is returned in a list for some reason. attr = attributes[ssh_opt] - if type(attr) is list: + if isinstance(attr, MutableSequence): attr = attr[0] tmp_dict[ans_opt] = attr if tmp_dict: diff --git a/lib/ansible/plugins/callback/dense.py b/lib/ansible/plugins/callback/dense.py index 22b70f3847..03098116a7 100644 --- a/lib/ansible/plugins/callback/dense.py +++ b/lib/ansible/plugins/callback/dense.py @@ -19,8 +19,7 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type -from ansible.plugins.callback.default import CallbackModule as CallbackModule_default -from ansible.utils.color import colorize, hostcolor +from collections import MutableMapping, MutableSequence HAS_OD = False try: @@ -29,6 +28,10 @@ try: except ImportError: pass +from ansible.module_utils.six import binary_type, text_type +from ansible.plugins.callback.default import CallbackModule as CallbackModule_default +from ansible.utils.color import colorize, hostcolor + try: from __main__ import display except ImportError: @@ -235,7 +238,7 @@ class CallbackModule_dense(CallbackModule_default): # Remove empty attributes (list, dict, str) for attr in result.copy(): - if type(result[attr]) in (list, dict, basestring, unicode): + if isinstance(result[attr], (MutableSequence, MutableMapping, binary_type, text_type)): if not result[attr]: del(result[attr]) diff --git a/lib/ansible/plugins/callback/log_plays.py b/lib/ansible/plugins/callback/log_plays.py index f82ebda9e1..0eb33afd01 100644 --- a/lib/ansible/plugins/callback/log_plays.py +++ b/lib/ansible/plugins/callback/log_plays.py @@ -22,6 +22,7 @@ __metaclass__ = type import os import time import json +from collections import MutableMapping from ansible.module_utils._text import to_bytes from ansible.plugins.callback import CallbackBase @@ -54,7 +55,7 @@ class CallbackModule(CallbackBase): os.makedirs("/var/log/ansible/hosts") def log(self, host, category, data): - if type(data) == dict: + if isinstance(data, MutableMapping): if '_ansible_verbose_override' in data: # avoid logging extraneous data data = 'omitted' diff --git a/lib/ansible/plugins/filter/core.py b/lib/ansible/plugins/filter/core.py index 77e961db8b..89b264bcd7 100644 --- a/lib/ansible/plugins/filter/core.py +++ b/lib/ansible/plugins/filter/core.py @@ -31,6 +31,7 @@ import re import string import sys import uuid +from collections import MutableMapping, MutableSequence from datetime import datetime from functools import partial from random import Random, SystemRandom, shuffle @@ -108,14 +109,13 @@ def to_nice_json(a, indent=4, *args, **kw): def to_bool(a): ''' return a bool for the arg ''' - if a is None or type(a) == bool: + if a is None or isinstance(a, bool): return a if isinstance(a, string_types): a = a.lower() - if a in ['yes', 'on', '1', 'true', 1]: + if a in ('yes', 'on', '1', 'true', 1): return True - else: - return False + return False def to_datetime(string, format="%Y-%d-%m %H:%M:%S"): return datetime.strptime(string, format) @@ -402,10 +402,10 @@ def extract(item, container, morekeys=None): def failed(*a, **kw): ''' Test if task result yields failed ''' item = a[0] - if type(item) != dict: + if not isinstance(item, MutableMapping): raise errors.AnsibleFilterError("|failed expects a dictionary") - rc = item.get('rc',0) - failed = item.get('failed',False) + rc = item.get('rc', 0) + failed = item.get('failed', False) if rc != 0 or failed: return True else: @@ -418,13 +418,13 @@ def success(*a, **kw): def changed(*a, **kw): ''' Test if task result yields changed ''' item = a[0] - if type(item) != dict: + if not isinstance(item, MutableMapping): raise errors.AnsibleFilterError("|changed expects a dictionary") if not 'changed' in item: changed = False if ('results' in item # some modules return a 'results' key - and type(item['results']) == list - and type(item['results'][0]) == dict): + and isinstance(item['results'], MutableSequence) + and isinstance(item['results'][0], MutableMapping)): for result in item['results']: changed = changed or result.get('changed', False) else: @@ -434,7 +434,7 @@ def changed(*a, **kw): def skipped(*a, **kw): ''' Test if task result yields skipped ''' item = a[0] - if type(item) != dict: + if not isinstance(item, MutableMapping): raise errors.AnsibleFilterError("|skipped expects a dictionary") skipped = item.get('skipped', False) return skipped diff --git a/lib/ansible/plugins/lookup/csvfile.py b/lib/ansible/plugins/lookup/csvfile.py index 195a69e56a..6279d8c081 100644 --- a/lib/ansible/plugins/lookup/csvfile.py +++ b/lib/ansible/plugins/lookup/csvfile.py @@ -19,6 +19,7 @@ __metaclass__ = type import codecs import csv +from collections import MutableSequence from ansible.errors import AnsibleError from ansible.plugins.lookup import LookupBase @@ -102,7 +103,7 @@ class LookupModule(LookupBase): lookupfile = self.find_file_in_search_path(variables, 'files', paramvals['file']) var = self.read_csv(lookupfile, key, paramvals['delimiter'], paramvals['encoding'], paramvals['default'], paramvals['col']) if var is not None: - if type(var) is list: + if isinstance(var, MutableSequence): for v in var: ret.append(v) else: diff --git a/lib/ansible/plugins/lookup/dig.py b/lib/ansible/plugins/lookup/dig.py index 252739afe5..e66eadfcfe 100644 --- a/lib/ansible/plugins/lookup/dig.py +++ b/lib/ansible/plugins/lookup/dig.py @@ -22,11 +22,12 @@ from ansible.plugins.lookup import LookupBase import socket try: + import dns.exception + import dns.name import dns.resolver import dns.reversename from dns.rdatatype import (A, AAAA, CNAME, DLV, DNAME, DNSKEY, DS, HINFO, LOC, MX, NAPTR, NS, NSEC3PARAM, PTR, RP, SOA, SPF, SRV, SSHFP, TLSA, TXT) - import dns.exception HAVE_DNS = True except ImportError: HAVE_DNS = False @@ -70,7 +71,7 @@ def make_rdata_dict(rdata): for f in fields: val = rdata.__getattribute__(f) - if type(val) == dns.name.Name: + if isinstance(val, dns.name.Name): val = dns.name.Name.to_text(val) if rdata.rdtype == DLV and f == 'digest': diff --git a/lib/ansible/plugins/lookup/ini.py b/lib/ansible/plugins/lookup/ini.py index 7f67fe06ff..040571cec1 100644 --- a/lib/ansible/plugins/lookup/ini.py +++ b/lib/ansible/plugins/lookup/ini.py @@ -17,9 +17,10 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type -from io import StringIO import os import re +from collections import MutableSequence +from io import StringIO from ansible.errors import AnsibleError from ansible.module_utils.six.moves import configparser @@ -110,7 +111,7 @@ class LookupModule(LookupBase): else: var = self.read_ini(path, key, paramvals['section'], paramvals['default'], paramvals['re']) if var is not None: - if type(var) is list: + if isinstance(var, MutableSequence): for v in var: ret.append(v) else: diff --git a/lib/ansible/plugins/strategy/__init__.py b/lib/ansible/plugins/strategy/__init__.py index fa6746b194..eeb429b9b0 100644 --- a/lib/ansible/plugins/strategy/__init__.py +++ b/lib/ansible/plugins/strategy/__init__.py @@ -53,6 +53,9 @@ except ImportError: __all__ = ['StrategyBase'] +class StrategySentinel: + pass + # TODO: this should probably be in the plugins/__init__.py, with # a smarter mechanism to set all of the attributes based on # the loaders created there @@ -70,12 +73,12 @@ class SharedPluginLoaderObj: self.module_loader = module_loader -_sentinel = object() +_sentinel = StrategySentinel() def results_thread_main(strategy): while True: try: result = strategy._final_q.get() - if type(result) == object: + if isinstance(result, StrategySentinel): break else: strategy._results_lock.acquire() diff --git a/lib/ansible/plugins/test/core.py b/lib/ansible/plugins/test/core.py index a8bdbf1c2b..59244dc8c1 100644 --- a/lib/ansible/plugins/test/core.py +++ b/lib/ansible/plugins/test/core.py @@ -21,6 +21,7 @@ __metaclass__ = type import re import operator as py_operator +from collections import MutableMapping, MutableSequence from distutils.version import LooseVersion, StrictVersion from ansible import errors @@ -28,7 +29,7 @@ from ansible import errors def failed(*a, **kw): ''' Test if task result yields failed ''' item = a[0] - if type(item) != dict: + if not isinstance(item, MutableMapping): raise errors.AnsibleFilterError("|failed expects a dictionary") rc = item.get('rc',0) failed = item.get('failed',False) @@ -44,13 +45,13 @@ def success(*a, **kw): def changed(*a, **kw): ''' Test if task result yields changed ''' item = a[0] - if type(item) != dict: + if not isinstance(item, MutableMapping): raise errors.AnsibleFilterError("|changed expects a dictionary") if not 'changed' in item: changed = False if ('results' in item # some modules return a 'results' key - and type(item['results']) == list - and type(item['results'][0]) == dict): + and isinstance(item['results'], MutableSequence) + and isinstance(item['results'][0], MutableMapping)): for result in item['results']: changed = changed or result.get('changed', False) else: @@ -60,7 +61,7 @@ def changed(*a, **kw): def skipped(*a, **kw): ''' Test if task result yields skipped ''' item = a[0] - if type(item) != dict: + if not isinstance(item, MutableMapping): raise errors.AnsibleFilterError("|skipped expects a dictionary") skipped = item.get('skipped', False) return skipped diff --git a/test/units/modules/cloud/openstack/test_os_server.py b/test/units/modules/cloud/openstack/test_os_server.py index 3ab3e40580..71082a9b2f 100644 --- a/test/units/modules/cloud/openstack/test_os_server.py +++ b/test/units/modules/cloud/openstack/test_os_server.py @@ -4,6 +4,7 @@ import yaml import inspect import collections +from ansible.module_utils.six import string_types from ansible.modules.cloud.openstack import os_server @@ -26,7 +27,7 @@ def params_from_doc(func): for task in cfg: for module, params in task.items(): for k, v in params.items(): - if k in ['nics'] and type(v) == str: + if k in ['nics'] and isinstance(v, string_types): params[k] = [v] task[module] = collections.defaultdict(str, params)