Merge pull request #1 from dgoodwin/pep8

Pylint fixes and ignores for incoming oo-install code.

6 files changed, 56 insertions, 30 deletions

diff --git a/utils/src/ooinstall/__init__.py b/utils/src/ooinstall/__init__.py
index acaac8d85..944dea3b5 100644
--- a/utils/src/ooinstall/__init__.py
+++ b/utils/src/ooinstall/__init__.py
@@ -1 +1,5 @@
+# TODO: Temporarily disabled due to importing old code into openshift-ansible
+# repo. We will work on these over time.
+# pylint: disable=missing-docstring
+
 from .oo_config import OOConfig
diff --git a/utils/src/ooinstall/ansible_plugins/facts_callback.py b/utils/src/ooinstall/ansible_plugins/facts_callback.py
index b19e6848e..ea6ed6574 100644
--- a/utils/src/ooinstall/ansible_plugins/facts_callback.py
+++ b/utils/src/ooinstall/ansible_plugins/facts_callback.py
@@ -1,20 +1,25 @@
+# TODO: Temporarily disabled due to importing old code into openshift-ansible
+# repo. We will work on these over time.
+# pylint: disable=bad-continuation,missing-docstring,no-self-use,invalid-name,no-value-for-parameter
+
 import os
 import yaml
-from tempfile import mkstemp
 
 class CallbackModule(object):
-    """
-    """
 
     def __init__(self):
         ######################
         # This is ugly stoopid. This should be updated in the following ways:
-        # 1) it should probably only be used for the openshift_facts.yml playbook, so maybe there's some way to check a variable that's set when that playbook is run?
+        # 1) it should probably only be used for the
+        # openshift_facts.yml playbook, so maybe there's some way to check
+        # a variable that's set when that playbook is run?
         try:
             self.hosts_yaml_name = os.environ['OO_INSTALL_CALLBACK_FACTS_YAML']
         except KeyError:
-            raise ValueError('The OO_INSTALL_CALLBACK_FACTS_YAML environment variable must be set.')
-        self.hosts_yaml = os.open(self.hosts_yaml_name, os.O_CREAT | os.O_WRONLY)
+            raise ValueError('The OO_INSTALL_CALLBACK_FACTS_YAML environment '
+                'variable must be set.')
+        self.hosts_yaml = os.open(self.hosts_yaml_name, os.O_CREAT |
+            os.O_WRONLY)
 
     def on_any(self, *args, **kwargs):
         pass
@@ -62,7 +67,9 @@ class CallbackModule(object):
     def playbook_on_task_start(self, name, is_conditional):
         pass
 
-    def playbook_on_vars_prompt(self, varname, private=True, prompt=None, encrypt=None, confirm=False, salt_size=None, salt=None, default=None):
+    #pylint: disable=too-many-arguments
+    def playbook_on_vars_prompt(self, varname, private=True, prompt=None,
+        encrypt=None, confirm=False, salt_size=None, salt=None, default=None):
         pass
 
     def playbook_on_setup(self):
diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py
index a23dc0d53..c2ae00bd1 100644
--- a/utils/src/ooinstall/cli_installer.py
+++ b/utils/src/ooinstall/cli_installer.py
@@ -1,3 +1,7 @@
+# TODO: Temporarily disabled due to importing old code into openshift-ansible
+# repo. We will work on these over time.
+# pylint: disable=bad-continuation,missing-docstring,no-self-use,invalid-name,no-value-for-parameter
+
 import click
 import os
 import re
@@ -5,11 +9,11 @@ import sys
 from ooinstall import install_transactions
 from ooinstall import OOConfig
 from ooinstall.oo_config import Host
-from variants import find_variant, get_variant_version_combos
+from ooinstall.variants import find_variant, get_variant_version_combos
 
 DEFAULT_ANSIBLE_CONFIG = '/usr/share/atomic-openshift-util/ansible.cfg'
 
-def validate_ansible_dir(ctx, param, path):
+def validate_ansible_dir(path):
     if not path:
         raise click.BadParameter('An ansible path must be provided')
     return path
@@ -20,19 +24,10 @@ def is_valid_hostname(hostname):
     if not hostname or len(hostname) > 255:
         return False
     if hostname[-1] == ".":
-        hostname = hostname[:-1] # strip exactly one dot from the right, if present
-    allowed = re.compile("(?!-)[A-Z\d-]{1,63}(?<!-)$", re.IGNORECASE)
+        hostname = hostname[:-1]  # strip exactly one dot from the right, if present
+    allowed = re.compile(r"(?!-)[A-Z\d-]{1,63}(?<!-)$", re.IGNORECASE)
     return all(allowed.match(x) for x in hostname.split("."))
 
-def validate_hostname(ctx, param, hosts):
-    # if '' == hostname or is_valid_hostname(hostname):
-    for hostname in hosts:
-        if not is_valid_hostname(hostname):
-            raise click.BadParameter('"{}" appears to be an invalid hostname. ' \
-                                     'Please double-check this value ' \
-                                     'and re-enter it.'.format(hostname))
-    return hosts
-
 def validate_prompt_hostname(hostname):
     if '' == hostname or is_valid_hostname(hostname):
         return hostname
@@ -402,6 +397,8 @@ def get_hosts_to_run_on(oo_cfg, callback_facts, unattended, force):
               default="/tmp/ansible.log")
 @click.option('--unattended', '-u', is_flag=True, default=False)
 @click.option('--force', '-f', is_flag=True, default=False)
+#pylint: disable=too-many-arguments
+# Main CLI entrypoint, not much we can do about too many arguments.
 def main(configuration, ansible_playbook_directory, ansible_config, ansible_log_path, unattended, force):
     oo_cfg = OOConfig(configuration)
 
@@ -414,7 +411,7 @@ def main(configuration, ansible_playbook_directory, ansible_config, ansible_log_
         # If we're installed by RPM this file should exist and we can use it as our default:
         oo_cfg.settings['ansible_config'] = DEFAULT_ANSIBLE_CONFIG
 
-    validate_ansible_dir(None, None, ansible_playbook_directory)
+    validate_ansible_dir(ansible_playbook_directory)
     oo_cfg.settings['ansible_playbook_directory'] = ansible_playbook_directory
     oo_cfg.ansible_playbook_directory = ansible_playbook_directory
 
@@ -444,7 +441,7 @@ def main(configuration, ansible_playbook_directory, ansible_config, ansible_log_
     # to confirm the settings for new nodes. Look into this once we're distinguishing
     # between new and pre-existing nodes.
     if len(oo_cfg.calc_missing_facts()) > 0:
-        validated_hosts = confirm_hosts_facts(oo_cfg, callback_facts)
+        confirm_hosts_facts(oo_cfg, callback_facts)
 
     oo_cfg.save_to_disk()
 
diff --git a/utils/src/ooinstall/install_transactions.py b/utils/src/ooinstall/install_transactions.py
index ac69bb4b8..cef6662d7 100644
--- a/utils/src/ooinstall/install_transactions.py
+++ b/utils/src/ooinstall/install_transactions.py
@@ -1,7 +1,11 @@
+# TODO: Temporarily disabled due to importing old code into openshift-ansible
+# repo. We will work on these over time.
+# pylint: disable=bad-continuation,missing-docstring,no-self-use,invalid-name,global-statement,global-variable-not-assigned
+
 import subprocess
 import os
 import yaml
-from variants import find_variant
+from ooinstall.variants import find_variant
 
 CFG = None
 
@@ -21,14 +25,19 @@ def generate_inventory(hosts):
         base_inventory.write('ansible_sudo=true\n')
 
     # Find the correct deployment type for ansible:
-    variant, ver = find_variant(CFG.settings['variant'],
-        version=CFG.settings.get('variant_version', None))
+    ver = find_variant(CFG.settings['variant'],
+        version=CFG.settings.get('variant_version', None))[1]
     base_inventory.write('deployment_type={}\n'.format(ver.ansible_key))
 
     if 'OO_INSTALL_DEVEL_REGISTRY' in os.environ:
-        base_inventory.write('oreg_url=rcm-img-docker01.build.eng.bos.redhat.com:5001/openshift3/ose-${component}:${version}\n')
+        base_inventory.write('oreg_url=rcm-img-docker01.build.eng.bos.redhat.com:'
+            '5001/openshift3/ose-${component}:${version}\n')
     if 'OO_INSTALL_PUDDLE_REPO_ENABLE' in os.environ:
-        base_inventory.write("openshift_additional_repos=[{'id': 'ose-devel', 'name': 'ose-devel', 'baseurl': 'http://buildvm-devops.usersys.redhat.com/puddle/build/AtomicOpenShift/3.1/latest/RH7-RHAOS-3.1/$basearch/os', 'enabled': 1, 'gpgcheck': 0}]\n")
+        base_inventory.write("openshift_additional_repos=[{'id': 'ose-devel', "
+            "'name': 'ose-devel', "
+            "'baseurl': 'http://buildvm-devops.usersys.redhat.com"
+            "/puddle/build/AtomicOpenShift/3.1/latest/RH7-RHAOS-3.1/$basearch/os', "
+            "'enabled': 1, 'gpgcheck': 0}]\n")
     if 'OO_INSTALL_STAGE_REGISTRY' in os.environ:
         base_inventory.write('oreg_url=registry.access.stage.redhat.com/openshift3/ose-${component}:${version}\n')
 
@@ -76,8 +85,8 @@ def load_system_facts(inventory_file, os_facts_path, env_vars):
     status = subprocess.call(['ansible-playbook',
                      '--inventory-file={}'.format(inventory_file),
                      os_facts_path],
-                     env=env_vars)
-                         #                     stdout=FNULL)
+                     env=env_vars,
+                     stdout=FNULL)
     if not status == 0:
         return [], 1
     callback_facts_file = open(CFG.settings['ansible_callback_facts_yaml'], 'r')
diff --git a/utils/src/ooinstall/oo_config.py b/utils/src/ooinstall/oo_config.py
index ea9638fe9..a2f53cf78 100644
--- a/utils/src/ooinstall/oo_config.py
+++ b/utils/src/ooinstall/oo_config.py
@@ -1,3 +1,7 @@
+# TODO: Temporarily disabled due to importing old code into openshift-ansible
+# repo. We will work on these over time.
+# pylint: disable=bad-continuation,missing-docstring,no-self-use,invalid-name,too-many-instance-attributes,too-few-public-methods
+
 import os
 import yaml
 from pkg_resources import resource_filename
@@ -123,7 +127,8 @@ class OOConfig(object):
             self.settings['ansible_plugins_directory'] = resource_filename(__name__, 'ansible_plugins')
 
         if 'ansible_callback_facts_yaml' not in self.settings:
-            self.settings['ansible_callback_facts_yaml'] = '{}/callback_facts.yaml'.format(self.settings['ansible_inventory_directory'])
+            self.settings['ansible_callback_facts_yaml'] = '%s/callback_facts.yaml' % \
+                self.settings['ansible_inventory_directory']
 
         if 'ansible_ssh_user' not in self.settings:
             self.settings['ansible_ssh_user'] = ''
diff --git a/utils/src/ooinstall/variants.py b/utils/src/ooinstall/variants.py
index a5159f857..ed98429fc 100644
--- a/utils/src/ooinstall/variants.py
+++ b/utils/src/ooinstall/variants.py
@@ -1,3 +1,7 @@
+# TODO: Temporarily disabled due to importing old code into openshift-ansible
+# repo. We will work on these over time.
+# pylint: disable=bad-continuation,missing-docstring,no-self-use,invalid-name,too-few-public-methods
+
 """
 Defines the supported variants and versions the installer supports, and metadata
 required to run Ansible correctly.