Skip to content

Commit

Permalink
Fix PyLint errors discovered when upgrading to newer version
Browse files Browse the repository at this point in the history
* Fixes PyLint to run in the virtualenv used for all tests

* Replaced 'LooseVersion' with 'parse_version' from setuptools
- This is a work around for the issue in
  pylint-dev/pylint#73 in which pylint can not
  import disutils.version correctly in a virtualenv.

* Removed the unused function 'delete_hosts' which was causing a
  pylint error as well

* Removed a deprecated pylint pragma option, 'bad-builtin'

* Fixed some import ordering issues it was picky about

* Added another disable for a case where the PyLint suggestion would
  have us altering the container we would be iterating over

* Add code-coverage reports to the unittests with the MINIMUM coverage
  percentage for success set to 70%
- Current test coverage is at 76%
  • Loading branch information
tbielawa committed Aug 26, 2016
1 parent 577195e commit c959f9d
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 32 deletions.
1 change: 1 addition & 0 deletions utils/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,4 @@ coverage.xml
docs/_build/
oo-install
oo-installenv
cover
12 changes: 8 additions & 4 deletions utils/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,17 @@ clean:
@rm -fR build dist rpm-build MANIFEST htmlcov .coverage cover ooinstall.egg-info oo-install
@rm -fR $(NAME)env

viewcover:
xdg-open cover/index.html

virtualenv:
@echo "#############################################"
@echo "# Creating a virtualenv"
@echo "#############################################"
virtualenv $(NAME)env
. $(NAME)env/bin/activate && pip install -r requirements.txt
. $(NAME)env/bin/activate && pip install pep8 nose coverage mock flake8 PyYAML click
. $(NAME)env/bin/activate && pip install setuptools --upgrade
. $(NAME)env/bin/activate && pip install enum configparser pylint pep8 nose coverage mock flake8 PyYAML click

# If there are any special things to install do it here
# . $(NAME)env/bin/activate && INSTALL STUFF
Expand All @@ -50,14 +54,14 @@ ci-unittests:
@echo "#############################################"
@echo "# Running Unit Tests in virtualenv"
@echo "#############################################"
# . $(NAME)env/bin/activate && nosetests -v --with-cover --cover-html --cover-min-percentage=80 --cover-package=$(TESTPACKAGE) test/
. $(NAME)env/bin/activate && nosetests -v test/
. $(NAME)env/bin/activate && nosetests -v --with-coverage --cover-html --cover-min-percentage=70 --cover-package=$(SHORTNAME) test/
@echo "VIEW CODE COVERAGE REPORT WITH 'xdg-open cover/index.html' or run 'make viewcover'"

ci-pylint:
@echo "#############################################"
@echo "# Running PyLint Tests in virtualenv"
@echo "#############################################"
python -m pylint --rcfile ../git/.pylintrc src/ooinstall/cli_installer.py src/ooinstall/oo_config.py src/ooinstall/openshift_ansible.py src/ooinstall/variants.py
. $(NAME)env/bin/activate && python -m pylint --rcfile ../git/.pylintrc src/ooinstall/cli_installer.py src/ooinstall/oo_config.py src/ooinstall/openshift_ansible.py src/ooinstall/variants.py

ci-list-deps:
@echo "#############################################"
Expand Down
32 changes: 7 additions & 25 deletions utils/src/ooinstall/cli_installer.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@
import os
import re
import sys
from distutils.version import LooseVersion
import logging
import setuptools.version
import click
from ooinstall import openshift_ansible
from ooinstall.oo_config import OOConfig
from ooinstall.oo_config import OOConfigInvalidHostError
from ooinstall.oo_config import Host, Role
from ooinstall.variants import find_variant, get_variant_version_combos

import logging
installer_log = logging.getLogger('installer')
installer_log.setLevel(logging.CRITICAL)
installer_file_handler = logging.FileHandler('/tmp/installer.txt')
Expand Down Expand Up @@ -98,27 +98,6 @@ def list_hosts(hosts):
click.echo(' {}: {}'.format(idx, hosts[idx]))


def delete_hosts(hosts):
while True:
list_hosts(hosts)
del_idx = click.prompt('Select host to delete, y/Y to confirm, '
'or n/N to add more hosts', default='n')
try:
del_idx = int(del_idx)
hosts.remove(hosts[del_idx])
except IndexError:
click.echo("\"{}\" doesn't match any hosts listed.".format(del_idx))
except ValueError:
try:
response = del_idx.lower()
if response in ['y', 'n']:
return hosts, response
click.echo("\"{}\" doesn't correspond to any valid input.".format(del_idx))
except AttributeError:
click.echo("\"{}\" doesn't correspond to any valid input.".format(del_idx))
return hosts, None


def collect_hosts(oo_cfg, existing_env=False, masters_set=False, print_summary=True):
"""
Collect host information from user. This will later be filled in using
Expand Down Expand Up @@ -652,8 +631,11 @@ def get_missing_info_from_user(oo_cfg):
oo_cfg.deployment.variables['master_routingconfig_subdomain'] = get_master_routingconfig_subdomain()
click.clear()

current_version = setuptools.version.pkg_resources.parse_version(
oo_cfg.settings.get('variant_version', '0.0'))
min_version = setuptools.version.pkg_resources.parse_version('3.2')
if not oo_cfg.settings.get('openshift_http_proxy', None) and \
LooseVersion(oo_cfg.settings.get('variant_version', '0.0')) >= LooseVersion('3.2'):
current_version >= min_version:
http_proxy, https_proxy, proxy_excludes = get_proxy_hostnames_and_excludes()
oo_cfg.deployment.variables['proxy_http'] = http_proxy
oo_cfg.deployment.variables['proxy_https'] = https_proxy
Expand Down Expand Up @@ -920,7 +902,7 @@ def uninstall(ctx):
@click.option('--latest-minor', '-l', is_flag=True, default=False)
@click.option('--next-major', '-n', is_flag=True, default=False)
@click.pass_context
# pylint: disable=bad-builtin,too-many-statements
# pylint: disable=too-many-statements
def upgrade(ctx, latest_minor, next_major):
oo_cfg = ctx.obj['oo_cfg']

Expand Down
7 changes: 6 additions & 1 deletion utils/src/ooinstall/oo_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@

import os
import sys
import logging
import yaml
from pkg_resources import resource_filename

import logging

installer_log = logging.getLogger('installer')

CONFIG_PERSIST_SETTINGS = [
Expand Down Expand Up @@ -325,6 +326,10 @@ def _set_defaults(self):
self.settings['ansible_inventory_path'] = \
'{}/hosts'.format(os.path.dirname(self.config_path))

# pylint: disable=consider-iterating-dictionary
# Disabled because we shouldn't alter the container we're
# iterating over
#
# clean up any empty sets
for setting in self.settings.keys():
if not self.settings[setting]:
Expand Down
5 changes: 3 additions & 2 deletions utils/src/ooinstall/openshift_ansible.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@
import subprocess
import sys
import os
import logging
import yaml
from ooinstall.variants import find_variant
import logging

installer_log = logging.getLogger('installer')

CFG = None
Expand Down Expand Up @@ -229,7 +230,7 @@ def load_system_facts(inventory_file, os_facts_path, env_vars, verbose=False):
os_facts_path])
installer_log.debug("Going to subprocess out to ansible now with these args: %s", ' '.join(args))
status = subprocess.call(args, env=env_vars, stdout=FNULL)
if not status == 0:
if status != 0:
installer_log.debug("Exit status from subprocess was not 0")
return [], 1

Expand Down

0 comments on commit c959f9d

Please sign in to comment.