Repository: ansible/tower-cli
Pull request #361 (2017-09-19 17:36:09)

Merge v1 branch into master


Patch:
diff --git a/.gitignore b/.gitignore
index 2bd76ed..a980a5f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -68,3 +68,13 @@ results.xml
 
 # Sphinx
 docs_wip/build/*
+
+# tower-cli v1 build
+tower_cli_v1/
+bin/tower-cli-v1
+setup_v1.py
+
+# tower-cli v2 build
+tower_cli_v2/
+bin/tower-cli-v2
+setup_v2.py
diff --git a/.travis.yml b/.travis.yml
index 250220f..3e49a1f 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,7 +1,6 @@
 sudo: false
 language: python
 python:
-    - "2.6"
     - "2.7"
     - "3.3"
     - "3.4"
diff --git a/HISTORY.rst b/HISTORY.rst
index 2f26888..af54d2a 100644
--- a/HISTORY.rst
+++ b/HISTORY.rst
@@ -1,6 +1,22 @@
 Release History
 ===============
 
+3.1.8 (2017-08-21)
+------------------
+
+- Fix bug of incomplete role membership lookup, preventing granting of roles
+- New method of installing tower-cli-v1, specific to API v1 use
+- Combine click parameters from multiple base classes in metaclass
+- Fix unicode bug in human display format
+- Add new page_size parameter to list view
+- Add scm_update_cache_timeout field to project resource
+- Begin process to deprecate python 2.6
+
+3.1.7 (2017-08-07)
+------------------
+
+- Follow up 3.1.6 by duplicating exceptions.py to support `import tower_cli.utils.exceptions` syntax.
+
 3.1.6 (2017-07-18)
 ------------------
 
diff --git a/Makefile b/Makefile
index 873c37c..4e0bdb8 100644
--- a/Makefile
+++ b/Makefile
@@ -1,4 +1,4 @@
-VERSION = $(shell cat VERSION)
+VERSION = $(shell cat tower_cli/VERSION)
 
 DISTS = el6 el7
 DIST_SUFFIX_el6 = 
@@ -46,3 +46,23 @@ rpm-build/.exists:
 install:
 	sudo rm -rf dist/ build/ 
 	sudo python setup.py install
+
+clean_v2:
+	rm -rf tower_cli_v2
+	rm -rf ansible_tower_cli_v2.egg-info
+	rm -rf setup_v2.py
+	rm -f bin/tower-cli-v2
+
+setup_v2.py:
+	cp -R tower_cli tower_cli_v2/
+	cp bin/tower-cli bin/tower-cli-v2
+	cp setup.py setup_v2.py
+	python version_swap.py
+
+prep_v2: setup_v2.py
+
+install_v2: setup_v2.py
+	sudo rm -rf dist/ build/
+	sudo python setup_v2.py install
+
+v2-refresh: clean_v2 install_v2
diff --git a/VERSION b/VERSION
deleted file mode 100644
index 9cec716..0000000
--- a/VERSION
+++ /dev/null
@@ -1,1 +0,0 @@
-3.1.6
diff --git a/packaging/rpm/ansible-tower-cli.spec b/packaging/rpm/ansible-tower-cli.spec
index 6ee4376..8cca643 100644
--- a/packaging/rpm/ansible-tower-cli.spec
+++ b/packaging/rpm/ansible-tower-cli.spec
@@ -2,7 +2,7 @@
 %global srcname tower-cli
 
 Name:           ansible-%{srcname}
-Version:        3.1.6
+Version:        3.1.8
 Release:        2%{?dist}
 Summary:        Commandline interface for Ansible Tower
 Group:          Development/Tools
diff --git a/setup.py b/setup.py
index 07df3e0..3d402d8 100644
--- a/setup.py
+++ b/setup.py
@@ -23,6 +23,9 @@ from setuptools import find_packages
 from setuptools.command.test import test as TestCommand
 
 
+pkg_name = 'tower_cli'
+
+
 class Tox(TestCommand):
     """The test command should install and then run tox.
 
@@ -112,8 +115,8 @@ def combine_files(*args):
 
 setup(
     # Basic metadata
-    name='ansible-tower-cli',
-    version=open('VERSION').read().strip(),
+    name='ansible-%s' % pkg_name.replace('_', '-'),
+    version=open('tower_cli/VERSION').read().strip(),
     author='Luke Sneeringer',
     author_email='lsneeringer@ansible.com',
     url='https://github.com/ansible/tower-cli',
@@ -126,10 +129,10 @@ setup(
     # How to do the install
     install_requires=parse_requirements('requirements.txt'),
     provides=[
-        'tower_cli',
+        pkg_name,
     ],
     scripts=[
-        'bin/tower-cli',
+        'bin/%s' % pkg_name.replace('_', '-'),
     ],
     packages=find_packages(exclude=['tests']),
     # How to do the tests
@@ -138,7 +141,7 @@ setup(
 
     # Data files
     package_data={
-        'tower_cli': ['VERSION'],
+        pkg_name: ['VERSION'],
     },
 
     # PyPI metadata.
diff --git a/tower_cli/VERSION b/tower_cli/VERSION
deleted file mode 120000
index 6ff19de..0000000
--- a/tower_cli/VERSION
+++ /dev/null
@@ -1,1 +0,0 @@
-../VERSION
\ No newline at end of file
diff --git a/tower_cli/VERSION b/tower_cli/VERSION
new file mode 100644
index 0000000..944880f
--- /dev/null
+++ b/tower_cli/VERSION
@@ -0,0 +1,1 @@
+3.2.0
diff --git a/tower_cli/cli/resource.py b/tower_cli/cli/resource.py
index ff3f91b..38df1ef 100644
--- a/tower_cli/cli/resource.py
+++ b/tower_cli/cli/resource.py
@@ -32,6 +32,11 @@ from tower_cli.cli.action import ActionSubcommand
 from tower_cli.exceptions import MultipleRelatedError
 from tower_cli.cli.types import StructuredInput
 
+try:
+    basestring
+except NameError:
+    basestring = None
+
 
 class ResSubcommand(click.MultiCommand):
     """A subcommand that implements all command methods on the
@@ -145,6 +150,17 @@ class ResSubcommand(click.MultiCommand):
         raise MultipleRelatedError(
             'Can not use id format when multiple objects are returned.')
 
+    @staticmethod
+    def get_print_value(data, col):
+        value = data.get(col, 'N/A')
+        is_bool = isinstance(value, bool)
+        if basestring and isinstance(value, basestring) and type(value) is not str:
+            value = value.encode('utf-8')  # handle python 2 encoding problem
+        value = '%s' % value
+        if is_bool:
+            value = value.lower()
+        return value
+
     def _format_human(self, payload):
         """Convert the payload into an ASCII table suitable for
         printing on screen and return it.
@@ -203,7 +219,7 @@ class ResSubcommand(click.MultiCommand):
         for col in columns:
             widths[col] = max(
                 len(col),
-                *[len(six.text_type(i.get(col, 'N/A'))) for i in raw_rows]
+                *[len(self.get_print_value(i, col)) for i in raw_rows]
             )
             fd = fields_by_name.get(col, None)
             if fd is not None and fd.col_width is not None:
@@ -235,10 +251,10 @@ class ResSubcommand(click.MultiCommand):
             data_row = ''
             for col in columns:
                 template = '{0:%d}' % widths[col]
-                value = raw_row.get(col, 'N/A')
-                if isinstance(raw_row.get(col, 'N/A'), bool):
+                value = self.get_print_value(raw_row, col)
+                # Right-align certain native data types
+                if isinstance(raw_row.get(col, 'N/A'), (bool, int)):
                     template = template.replace('{0:', '{0:>')
-                    value = six.text_type(value).lower()
                 # Truncate the cell entry if exceeds manually
                 # specified column width limit
                 fd = fields_by_name.get(col, None)
diff --git a/tower_cli/models/base.py b/tower_cli/models/base.py
index cb76430..c1d6cc2 100644
--- a/tower_cli/models/base.py
+++ b/tower_cli/models/base.py
@@ -63,16 +63,32 @@ class ResourceMeta(type):
             # the superclass implementation down to the subclass implementation.
             if not len(bases):
                 continue
-            superclass = bases[0]
-            super_method = getattr(superclass, key, None)
-            if super_method and getattr(super_method, '_cli_command', False):
-                # Copy the click parameters from the parent method to the child.
-                cp = getattr(value, '__click_params__', [])
-                cp = getattr(super_method, '__click_params__', []) + cp
-                value.__click_params__ = cp
-
-                # Copy the command attributes from the parent to the child, if the child has not overridden them.
-                for attkey, attval in super_method._cli_command_attrs.items():
+            cp = []
+            baseattrs = {}
+            for superclass in bases:
+                super_method = getattr(superclass, key, None)
+                if super_method and getattr(super_method, '_cli_command', False):
+                    # Copy the click parameters from the parent method to the child.
+                    for param in getattr(super_method, '__click_params__', []):
+                        if param not in cp:
+                            cp.append(param)
+
+                    # Copy the command attributes from the parent to the child, if the child has not overridden them.
+                    for attkey, attval in getattr(super_method, '_cli_command_attrs', {}).items():
+                        baseattrs.setdefault(attkey, attval)
+            if cp:
+                # If subclass method is not decorated as command, but parent
+                # classes do, then make it into a command here
+                if not hasattr(value, '__click_params__'):
+                    value.__click_params__ = []
+                if not hasattr(value, '_cli_command_attrs'):
+                    value._cli_command_attrs = {}
+
+                # Copy all parent click parameters to subclass method here
+                for param in cp:
+                    if param not in value.__click_params__:
+                        value.__click_params__.append(param)
+                for attkey, attval in baseattrs.items():
                     value._cli_command_attrs.setdefault(attkey, attval)
 
         disabled_methods = attrs.pop('disabled_methods', set())
@@ -454,6 +470,8 @@ class BaseResource(six.with_metaclass(ResourceMeta)):
                   help='If set, collate all pages of content from the API when returning results.')
     @click.option('--page', default=1, type=int, show_default=True,
                   help='The page to show. Ignored if --all-pages is sent.')
+    @click.option('--page-size', type=int, show_default=True, required=False,
+                  help='Number of records to show. Ignored if --all-pages.')
     @click.option('-Q', '--query', required=False, nargs=2, multiple=True,
                   help='A key and value to be passed as an HTTP query string key and value to the Tower API.'
                        ' Will be run through HTTP escaping. This argument may be sent multiple times.\n'
@@ -483,6 +501,7 @@ class BaseResource(six.with_metaclass(ResourceMeta)):
         # If the `all_pages` flag is set, then ignore any page that might also be sent.
         if all_pages:
             kwargs.pop('page', None)
+            kwargs.pop('page_size', None)
 
         # Get the response.
         debug.log('Getting records.', header='details')
diff --git a/tower_cli/resources/project.py b/tower_cli/resources/project.py
index a3dbc92..d343120 100644
--- a/tower_cli/resources/project.py
+++ b/tower_cli/resources/project.py
@@ -54,6 +54,7 @@ class Resource(models.Resource, models.MonitorableResource):
                                         display=False)
     scm_update_on_launch = models.Field(type=bool, required=False,
                                         display=False)
+    scm_update_cache_timeout = models.Field(type=int, required=False, display=False)
     job_timeout = models.Field(type=int, required=False, display=False,
                                help_text='The timeout field (in seconds).')
 
diff --git a/tower_cli/resources/role.py b/tower_cli/resources/role.py
index 24d3bdf..91124b2 100644
--- a/tower_cli/resources/role.py
+++ b/tower_cli/resources/role.py
@@ -251,6 +251,7 @@ class Resource(models.Resource):
         debug.log('Checking if %s already has role.' % obj_type,
                   header='details')
         data, self.endpoint = self.data_endpoint(kwargs)
+        data['content_type__model'] = res_type.replace('_', '')
         response = self.read(pk=None, fail_on_no_results=False,
                              fail_on_multiple_results=False, **data)
 
diff --git a/tower_cli/utils/exceptions.py b/tower_cli/utils/exceptions.py
new file mode 100644
index 0000000..21001f6
--- /dev/null
+++ b/tower_cli/utils/exceptions.py
@@ -0,0 +1,151 @@
+# Copyright 2015, Ansible, Inc.
+# Luke Sneeringer <lsneeringer@ansible.com>
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+# NOTE: this is a duplicate of tower_cli/exceptions.py for backward compatibility
+# with Ansible Tower modules.
+
+import click
+from click._compat import get_text_stderr
+
+
+class TowerCLIError(click.ClickException):
+    """Base exception class for problems raised within Tower CLI.
+    This class adds coloring to exceptions.
+    """
+    fg = 'red'
+    bg = None
+    bold = True
+
+    def show(self, file=None):
+        if file is None:
+            file = get_text_stderr()
+        click.secho('Error: %s' % self.format_message(), file=file,
+                    fg=self.fg, bg=self.bg, bold=self.bold)
+
+
+class UsageError(TowerCLIError):
+    """An exception class for reporting usage errors.
+
+    This uses an exit code of 2 in order to match click (which matters more
+    than following the erstwhile "standard" of using 64).
+    """
+    exit_code = 2
+
+
+class BadRequest(TowerCLIError):
+    """An exception class for reporting unexpected error codes from Ansible
+    Tower such that 400 <= code < 500.
+
+    In theory, we should never, ever get these.
+    """
+    exit_code = 40
+
+
+class AuthError(TowerCLIError):
+    """An exception class for reporting when a request failed due to an
+    authorization failure.
+    """
+    exit_code = 41
+
+
+class Forbidden(TowerCLIError):
+    """An exception class for reporting when a user doesn't have permission
+    to do something.
+    """
+    exit_code = 43
+
+
+class NotFound(TowerCLIError):
+    """An exception class for reporting when a request went through without
+    incident, but the requested content could not be found.
+    """
+    exit_code = 44
+
+
+class MethodNotAllowed(BadRequest):
+    """An exception class for sending a request to a URL where the URL doesn't
+    accept that method at all.
+    """
+    exit_code = 45
+
+
+class MultipleResults(TowerCLIError):
+    """An exception class for reporting when a request that expected one
+    and exactly one result got more than that.
+    """
+    exit_code = 49
+
+
+class ServerError(TowerCLIError):
+    """An exception class for reporting server-side errors which are expected
+    to be ephemeral.
+    """
+    exit_code = 50
+
+
+class Found(TowerCLIError):
+    """An exception class for when a record already exists, and we were
+    explicitly told that it shouldn't.
+    """
+    exit_code = 60
+
+
+class RelatedError(TowerCLIError):
+    """An exception class for errors where we can't find related objects
+    that we expect to find.
+    """
+    exit_code = 61
+
+
+class MultipleRelatedError(RelatedError):
+    """An exception class for errors where we try to find a single related
+    object, and get more than one.
+    """
+    exit_code = 62
+
+
+class ValidationError(TowerCLIError):
+    """An exception class for invalid values being sent as option
+    switches to Tower CLI.
+    """
+    exit_code = 64
+
+
+class CannotStartJob(TowerCLIError):
+    """An exception class for jobs that cannot be started within Tower
+    for whatever reason.
+    """
+    exit_code = 97
+
+
+class Timeout(TowerCLIError):
+    """An exception class for timeouts encountered within Tower CLI,
+    usually for monitoring.
+    """
+    exit_code = 98
+
+
+class JobFailure(TowerCLIError):
+    """An exception class for job failures that require error codes within
+    the Tower CLI.
+    """
+    exit_code = 99
+
+
+class ConnectionError(TowerCLIError):
+    """An exception class to bubble requests errors more nicely,
+    and communicate connection issues to the user.
+    """
+    exit_code = 120
diff --git a/version_swap.py b/version_swap.py
new file mode 100644
index 0000000..897b4bf
--- /dev/null
+++ b/version_swap.py
@@ -0,0 +1,29 @@
+import os
+
+
+API_VERSION = 2
+package_name = 'tower_cli_v%s' % API_VERSION
+
+
+def convert_file(filename):
+    with open(filename) as f:
+        s = f.read()
+    if package_name in s:
+        raise Exception(
+            'While attempting to convert %s. '
+            'Command has already ran, no need to run again.' % filename
+        )
+    s = s.replace('tower_cli', package_name)
+    s = s.replace('tower-cli', 'tower-cli-v%s' % API_VERSION)
+    with open(filename, "w") as f:
+        f.write(s)
+
+
+for dname, dirs, files in os.walk(package_name):
+    for fname in files:
+        fpath = os.path.join(dname, fname)
+        convert_file(fpath)
+
+
+convert_file('setup_v%s.py' % API_VERSION)
+convert_file('bin/tower-cli-v%s' % API_VERSION)


Test patch:
diff --git a/tests/test_cli_resource.py b/tests/test_cli_resource.py
index c81ab40..8fec368 100644
--- a/tests/test_cli_resource.py
+++ b/tests/test_cli_resource.py
@@ -1,3 +1,4 @@
+# -*- coding: utf-8 -*-
 import json
 import yaml
 
@@ -10,6 +11,11 @@ from tower_cli.conf import settings
 
 from tests.compat import unittest, mock
 
+try:
+    basestring
+except NameError:
+    basestring = None
+
 
 class SubcommandTests(unittest.TestCase):
     """A set of tests for establishing that the Subcommand class created
@@ -290,6 +296,26 @@ class SubcommandTests(unittest.TestCase):
         self.assertIn('1 Durham, NC', output)
         self.assertIn('2 Austin, TX', output)
 
+    def test_unicode_human_formatting(self):
+        value = 'unicode ❤ ☀ ☆ ☂'
+        data = {
+            'count': 1,
+            'results': [
+                {'id': 1, 'name': 'ascii'},
+                {
+                    'id': 42,
+                    'name': ''
+                }
+            ]
+        }
+        if basestring:
+            # Python < 3
+            data['results'][1]['name'] = value.decode('utf-8')
+        else:
+            data['results'][1]['name'] = value
+        output = self.command._format_human(data)
+        assert value in output
+
     def test_echo_method_human_formatted_changed(self):
         """Establish that if there is a change and no id is returned,
         we print a generic OK message.
diff --git a/tests/test_models_base.py b/tests/test_models_base.py
index 74aa0bf..c105bf0 100644
--- a/tests/test_models_base.py
+++ b/tests/test_models_base.py
@@ -91,8 +91,46 @@ class ResourceMetaTests(unittest.TestCase):
 
         # Establish that it has one of the options added to the
         # superclass list command.
-        self.assertTrue(any([i.name == 'query'
-                            for i in MyResource.list.__click_params__]))
+        self.assertEqual(MyResource.list.__click_params__, models.Resource.list.__click_params__)
+
+    def test_multiple_inheritance(self):
+        """
+        Establish that click decoration from all parent class chains are
+        preserved in a subclass.
+        """
+        class MyMixin(models.Resource):
+            endpoint = '/bogus/'
+
+            def list(self, **kwargs):
+                return super(MyMixin, self).list(**kwargs)
+
+        class MyResource(MyMixin, models.Resource):
+            endpoint = '/bogus/'
+
+            def list(self, **kwargs):
+                return super(MyResource, self).list(**kwargs)
+
+        self.assertTrue(hasattr(MyResource.list, '__click_params__'))
+        self.assertEqual(MyResource.list.__click_params__, models.Resource.list.__click_params__)
+
+    def test_no_duplicate_options_from_inheritance(self):
+        """
+        Test that metaclass does not duplicate options from multiple parents
+        """
+        class MyMixin1(models.Resource):
+            endpoint = '/bogus/'
+
+        class MyMixin2(models.Resource):
+            endpoint = '/boguser/'
+
+        class MyResource(MyMixin1, MyMixin2):
+            endpoint = '/boguser/'
+
+            def list(self, **kwargs):
+                return super(MyResource, self).list(**kwargs)
+
+        self.assertTrue(hasattr(MyResource.list, '__click_params__'))
+        self.assertEqual(MyResource.list.__click_params__, models.Resource.list.__click_params__)
 
     def test_fields(self):
         """Establish that fields are appropriately classified within