jjb/python-jenkins
Code Issues Proposed changes

Pass form data as body instead of in the url

Browse Source 
Switch create node to pass form data via request body, removing the
need for special handling in the _build_url method. Consequently remove
the string placeholder in the endpoint format string.

Remove unnecessary parameters of name and type from inclusion in the
json form data item, confirming they are ignored and required to be set
in the main form data.

Tested against jenkins 2.19.4.

Change-Id: I5f8f870a2ee3539505e984abccb0c87c632c1b9a
This commit is contained in:
Darragh Bailey 2017-06-16 12:12:31 +01:00
parent 16007e0185
commit cd237fbcad
2 changed files with 9 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
jenkins/__init__.py
View File

@ -110,7 +110,7 @@ BUILD_WITH_PARAMS_JOB = '%(folder_url)sjob/%(short_name)s/buildWithParameters'
BUILD_INFO = '%(folder_url)sjob/%(short_name)s/%(number)d/api/json?depth=%(depth)s'
BUILD_CONSOLE_OUTPUT = '%(folder_url)sjob/%(short_name)s/%(number)d/consoleText'
NODE_LIST = 'computer/api/json'
CREATE_NODE = 'computer/doCreateItem?%s'
CREATE_NODE = 'computer/doCreateItem'
DELETE_NODE = 'computer/%(name)s/doDelete'
NODE_INFO = 'computer/%(name)s/api/json?depth=%(depth)s'
NODE_TYPE = 'hudson.slaves.DumbSlave$DescriptorImpl'
@ -286,10 +286,7 @@ class Jenkins(object):
def _build_url(self, format_spec, variables=None):
if variables:
if format_spec == CREATE_NODE:
url_path = format_spec % urlencode(variables)
else:
url_path = format_spec % self._get_encoded_params(variables)
url_path = format_spec % self._get_encoded_params(variables)
else:
url_path = format_spec
@ -1281,13 +1278,11 @@ class Jenkins(object):
launcher_params['stapler-class'] = launcher
inner_params = {
'name': name,
'nodeDescription': nodeDescription,
'numExecutors': numExecutors,
'remoteFS': remoteFS,
'labelString': labels,
'mode': mode,
'type': NODE_TYPE,
'retentionStrategy': {
'stapler-class':
'hudson.slaves.RetentionStrategy$Always'
@ -1303,7 +1298,8 @@ class Jenkins(object):
}
self.jenkins_open(Request(
self._build_url(CREATE_NODE, params), b''))
self._build_url(CREATE_NODE, locals()),
urlencode(params).encode('utf-8')))
self.assert_node_exists(name, 'create[%s] failed')

19
tests/test_node.py
View File

@ -223,7 +223,8 @@ class JenkinsCreateNodeTest(JenkinsNodesTestBase):
'host': 'my.jenkins.slave1'
}
self.j.create_node(
'slave1',
# Note the use of a URL-encodable character "+" here.
'10.0.0.1+test-node',
nodeDescription='my test slave',
remoteFS='/home/juser',
labels='precise',
@ -231,11 +232,13 @@ class JenkinsCreateNodeTest(JenkinsNodesTestBase):
launcher=jenkins.LAUNCHER_SSH,
launcher_params=params)
actual = jenkins_mock.call_args_list[1][0][0].get_full_url()
actual = jenkins_mock.call_args_list[1][0][0].data.decode('utf-8')
# As python dicts do not guarantee order so the parameters get
# re-ordered when it gets processed by _get_encoded_params(),
# verify sections of the URL with self.assertIn() instead of
# the entire URL
self.assertIn(u'name=10.0.0.1%2Btest-node', actual)
self.assertIn(u'type=hudson.slaves.DumbSlave%24DescriptorImpl', actual)
self.assertIn(u'username%22%3A+%22juser', actual)
self.assertIn(
u'stapler-class%22%3A+%22hudson.plugins.sshslaves.SSHLauncher',
@ -247,20 +250,8 @@ class JenkinsCreateNodeTest(JenkinsNodesTestBase):
self.assertIn(u'port%22%3A+%2222', actual)
self.assertIn(u'remoteFS%22%3A+%22%2Fhome%2Fjuser', actual)
self.assertIn(u'labelString%22%3A+%22precise', actual)
self.assertIn(u'name%22%3A+%22slave1', actual)
self.assertIn(
u'type%22%3A+%22hudson.slaves.DumbSlave%24DescriptorImpl',
actual)
self._check_requests(jenkins_mock.call_args_list)
def test_build_url(self):
j = jenkins.Jenkins('https://test.noexist/')
# Note the use of a URL-encodable character "+" here.
variables = {'name': '10.0.0.1+test-node'}
result = j._build_url(jenkins.CREATE_NODE, variables=variables)
expected = 'https://test.noexist/computer/doCreateItem?name=10.0.0.1%2Btest-node'
self.assertEqual(result, expected)
@patch.object(jenkins.Jenkins, 'jenkins_open')
def test_already_exists(self, jenkins_mock):
jenkins_mock.side_effect = [