Repository: ansible/tower-cli (python)

Issue:
[v2] Remove `--use-job-endpoint` in job launch
it is deprecated in API v2, so it wouldn't work anyway

Pull request:
remove the unusable job endpoint launch method
Resolves #357

Patch:
diff --git a/tower_cli/resources/job.py b/tower_cli/resources/job.py
index 38ffc44..cd95f41 100644
--- a/tower_cli/resources/job.py
+++ b/tower_cli/resources/job.py
@@ -14,7 +14,6 @@
 # limitations under the License.
 
 from __future__ import absolute_import, unicode_literals
-from copy import copy
 from getpass import getpass
 from distutils.version import LooseVersion
 
@@ -85,9 +84,6 @@ class Resource(models.ExeResource):
         '--credential', required=False, type=types.Related('credential'),
         help='Specify machine credential for job template to run.'
     )
-    @click.option('--use-job-endpoint', required=False, default=False,
-                  is_flag=True, help='A flag that disable launching jobs'
-                  ' from job template when set.')
     def launch(self, job_template=None, monitor=False, wait=False,
                timeout=None, no_input=True, extra_vars=None, **kwargs):
         """Launch a new job based on a job template.
@@ -124,8 +120,6 @@ class Resource(models.ExeResource):
         :type inventory: str
         :param credential: Specify machine credential for job template to run.
         :type credential: str
-        :param use_job_endpoint: Flag that if set, disable launching jobs from job template when set.
-        :type use_job_endpoint: bool
         :returns: Result of subsequent ``monitor`` call if ``monitor`` flag is on; Result of subsequent
                   ``wait`` call if ``wait`` flag is on; Result of subsequent ``status`` call if none of
                   the two flags are on.
@@ -137,15 +131,12 @@ class Resource(models.ExeResource):
         # This is used as the baseline for starting the job.
 
         tags = kwargs.get('tags', None)
-        use_job_endpoint = kwargs.pop('use_job_endpoint', False)
         jt_resource = get_resource('job_template')
         jt = jt_resource.get(job_template)
 
         # Update the job data by adding an automatically-generated job name,
         # and removing the ID.
-        data = copy(jt)
-        data['job_template'] = data.pop('id')
-        data['name'] = '%s [invoked via. Tower CLI]' % data['name']
+        data = {}
         if tags:
             data['job_tags'] = tags
 
@@ -165,11 +156,11 @@ class Resource(models.ExeResource):
 
         # If the job template requires prompting for extra variables,
         # do so (unless --no-input is set).
-        if data.pop('ask_variables_on_launch', False) and not no_input \
+        if jt.get('ask_variables_on_launch', False) and not no_input \
                 and not extra_vars:
             # If JT extra_vars are JSON, echo them to user as YAML
             initial = parser.process_extra_vars(
-                [data['extra_vars']], force_json=False
+                [jt['extra_vars']], force_json=False
             )
             initial = '\n'.join((
                 '# Specify extra variables (if any) here as YAML.',
@@ -187,17 +178,14 @@ class Resource(models.ExeResource):
         # Replace/populate data fields if prompted.
         modified = set()
         for resource in PROMPT_LIST:
-            if data.pop('ask_' + resource + '_on_launch', False) \
-               and not no_input or use_job_endpoint:
+            if jt.pop('ask_' + resource + '_on_launch', False) and not no_input:
                 resource_object = kwargs.get(resource, None)
                 if type(resource_object) == types.Related:
                     resource_class = get_resource(resource)
-                    resource_object = resource_class.get(resource).\
-                        pop('id', None)
+                    resource_object = resource_class.get(resource).pop('id', None)
                 if resource_object is None:
-                    if not use_job_endpoint:
-                        debug.log('{0} is asked at launch but not provided'.
-                                  format(resource), header='warning')
+                    debug.log('{0} is asked at launch but not provided'.
+                              format(resource), header='warning')
                 elif resource != 'tags':
                     data[resource] = resource_object
                     modified.add(resource)
@@ -208,29 +196,16 @@ class Resource(models.ExeResource):
                 extra_vars_list, force_json=True
             )
 
-        # In Tower 2.1 and later, we create the new job with
-        # /job_templates/N/launch/; in Tower 2.0 and before, there is a two
-        # step process of posting to /jobs/ and then /jobs/N/start/.
-        supports_job_template_launch = False
-        if 'launch' in jt['related']:
-            supports_job_template_launch = True
-
         # Create the new job in Ansible Tower.
         start_data = {}
-        if supports_job_template_launch and not use_job_endpoint:
-            endpoint = '/job_templates/%d/launch/' % jt['id']
-            if 'extra_vars' in data and len(data['extra_vars']) > 0:
-                start_data['extra_vars'] = data['extra_vars']
-            if tags:
-                start_data['job_tags'] = data['job_tags']
-            for resource in PROMPT_LIST:
-                if resource in modified:
-                    start_data[resource] = data[resource]
-        else:
-            debug.log('Creating the job.', header='details')
-            job = client.post('/jobs/', data=data).json()
-            job_id = job['id']
-            endpoint = '/jobs/%d/start/' % job_id
+        endpoint = '/job_templates/%d/launch/' % jt['id']
+        if 'extra_vars' in data and len(data['extra_vars']) > 0:
+            start_data['extra_vars'] = data['extra_vars']
+        if tags:
+            start_data['job_tags'] = data['job_tags']
+        for resource in PROMPT_LIST:
+            if resource in modified:
+                start_data[resource] = data[resource]
 
         # There's a non-trivial chance that we are going to need some
         # additional information to start the job; in particular, many jobs
@@ -249,10 +224,8 @@ class Resource(models.ExeResource):
         kwargs.update(start_data)
         job_started = client.post(endpoint, data=kwargs)
 
-        # If this used the /job_template/N/launch/ route, get the job
-        # ID from the result.
-        if supports_job_template_launch and not use_job_endpoint:
-            job_id = job_started.json()['job']
+        # Get the job ID from the result.
+        job_id = job_started.json()['id']
 
         # If returning json indicates any ignored fields, display it in
         # verbose mode.


Test patch:
diff --git a/tests/test_resources_job.py b/tests/test_resources_job.py
index ca7a5d7..e45c53b 100644
--- a/tests/test_resources_job.py
+++ b/tests/test_resources_job.py
@@ -24,7 +24,6 @@ import tower_cli
 from tower_cli.api import client
 from tower_cli import exceptions as exc
 from tower_cli.cli.resource import ResSubcommand
-from tower_cli.constants import CUR_API_VERSION
 
 from tests.compat import unittest, mock
 from tower_cli.conf import settings
@@ -61,7 +60,7 @@ def standard_registration(t, **kwargs):
 
     # A POST to the launch endpoint will launch a job, and we
     # expect that the tower server will return the job number
-    data = {'job': 42}
+    data = {'id': 42}
     data.update(kwargs)
     t.register_json('/job_templates/1/launch/', data, method='POST')
 
@@ -82,7 +81,7 @@ def jt_vars_registration(t, extra_vars):
     register_get(t)
     t.register_json('/config/', {'version': '2.2.0'}, method='GET')
     t.register_json('/job_templates/1/launch/', {}, method='GET')
-    t.register_json('/job_templates/1/launch/', {'job': 42},
+    t.register_json('/job_templates/1/launch/', {'id': 42},
                     method='POST')
 
 
@@ -161,7 +160,7 @@ class LaunchTests(unittest.TestCase):
                 )
             self.assertDictContainsSubset(
                 {'foo': 'bar'},
-                json.loads(json.loads(t.requests[3].body)['extra_vars'])
+                json.loads(json.loads(t.requests[2].body)['extra_vars'])
             )
             self.assertDictContainsSubset({'changed': True, 'id': 42}, result)
 
@@ -200,63 +199,20 @@ class LaunchTests(unittest.TestCase):
                 self.assertEqual(t.requests[2].method, 'POST')
                 self.assertEqual(t.requests[2].body, '{}')
 
-    def test_job_template_variables(self):
-        """Establish that job template extra_vars are combined with local
-        extra vars, but only for older versions
-        """
-        with client.test_mode as t:
-            jt_vars_registration(t, 'spam: eggs')
-            result = self.res.launch(1, extra_vars=['foo: bar'])
-            response_json = yaml.load(t.requests[3].body)
-            ev_json = yaml.load(response_json['extra_vars'])
-            self.assertTrue('foo' in ev_json)
-            self.assertTrue('spam' in ev_json)
-            self.assertDictContainsSubset({'changed': True, 'id': 42}, result)
-
     def test_job_template_variables_post_24(self):
-        """ Check that in recent versions, it does not include job template
-         variables along with the rest """
+        """ Check that in Tower versions past 2.4,
+        it does not include job template
+        variables along with the rest """
         with client.test_mode as t:
             jt_vars_registration(t, 'spam: eggs')
             t.register_json('/config/', {'version': '2.4'}, method='GET')
             result = self.res.launch(1, extra_vars=['foo: bar'])
-            response_json = yaml.load(t.requests[3].body)
+            response_json = yaml.load(t.requests[2].body)
             ev_json = yaml.load(response_json['extra_vars'])
             self.assertTrue('foo' in ev_json)
             self.assertTrue('spam' not in ev_json)
             self.assertDictContainsSubset({'changed': True, 'id': 42}, result)
 
-    def test_extra_vars_at_runtime_tower_20(self):
-        """Establish that if we should be asking for extra variables at
-        runtime, that we do.
-        (This test is intended for Tower 2.0 compatibility.)
-        """
-        with client.test_mode as t:
-            t.register_json('/job_templates/1/', {
-                'ask_variables_on_launch': True,
-                'extra_vars': 'spam: eggs',
-                'id': 1,
-                'name': 'frobnicate',
-                'related': {},
-            })
-            register_get(t)
-            t.register_json('/config/', {'version': '2.0'}, method='GET')
-            t.register_json('/jobs/', {'id': 42}, method='POST')
-            t.register_json('/jobs/42/start/', {}, method='GET')
-            t.register_json('/jobs/42/start/', {}, method='POST')
-            with mock.patch.object(click, 'edit') as edit:
-                edit.return_value = '# Nothing.\nfoo: bar'
-                result = self.res.launch(1, no_input=False)
-                self.assertDictContainsSubset(
-                    {"spam": "eggs"},
-                    yaml.load(edit.mock_calls[0][1][0])
-                )
-            self.assertDictContainsSubset(
-                {'foo': 'bar'},
-                json.loads(json.loads(t.requests[2].body)['extra_vars'])
-            )
-            self.assertDictContainsSubset({'changed': True, 'id': 42}, result)
-
     def test_extra_vars_at_call_time(self):
         """Establish that extra variables specified at call time are
         appropriately specified.
@@ -269,7 +225,7 @@ class LaunchTests(unittest.TestCase):
             })
             register_get(t)
             t.register_json('/job_templates/1/launch/', {}, method='GET')
-            t.register_json('/job_templates/1/launch/', {'job': 42},
+            t.register_json('/job_templates/1/launch/', {'id': 42},
                             method='POST')
             result = self.res.launch(1, extra_vars=['foo: bar'])
 
@@ -291,7 +247,7 @@ class LaunchTests(unittest.TestCase):
             })
             register_get(t)
             t.register_json('/job_templates/1/launch/', {}, method='GET')
-            t.register_json('/job_templates/1/launch/', {'job': 42},
+            t.register_json('/job_templates/1/launch/', {'id': 42},
                             method='POST')
             result = self.res.launch(1, extra_vars=['foo: bar'])
 
@@ -315,7 +271,7 @@ class LaunchTests(unittest.TestCase):
             t.register_json('/job_templates/1/launch/', {
                 'passwords_needed_to_start': ['foo'],
             }, method='GET')
-            t.register_json('/job_templates/1/launch/', {'job': 42},
+            t.register_json('/job_templates/1/launch/', {'id': 42},
                             method='POST')
 
             with mock.patch('tower_cli.resources.job.getpass') as getpass:
@@ -324,26 +280,6 @@ class LaunchTests(unittest.TestCase):
                 getpass.assert_called_once_with('Password for foo: ')
             self.assertDictContainsSubset({'changed': True, 'id': 42}, result)
 
-    def test_launch_with_use_job_endpoint_set(self):
-        """Establish that launching a job with `--use-job-endpoint` set would
-        launch job through /jobs/\d/ endpoint regardless of the existence of
-        /job_templates/\d/launch/ endpoint
-        """
-        jt = {
-            'related': {
-                'launch': '/api/%s/job_templates/1/launch/' % CUR_API_VERSION
-            },
-            'id': 1,
-            'name': 'demo'
-        }
-        with client.test_mode as t:
-            t.register_json('/job_templates/1/', jt)
-            t.register_json('/jobs/16/', {})
-            t.register_json('/jobs/', {'id': 16}, method='POST')
-            t.register_json('/jobs/16/start/', {})
-            t.register_json('/jobs/16/start/', {}, method='POST')
-            self.res.launch(job_template=1, use_job_endpoint=True)
-
     def test_ignored_fields(self):
         """Establish that if ignored_fields is returned when launching job,
         it will be displayed in verbose mode.