jjb
/
python-jenkins
Code Issues Proposed changes

have get_guild_artifact support non-json files 

remove the json parsing from get_build_artifact to download artifacts not json-formatted
and use a stream to download binary artifacts

all artifacts are returned as binary string to avoid encoding issues

users wishing to parse json artifacts will need to do it outside of this method
(alternative could have been to keep get_build_artifact for json and add
 a new get_binary_build_artifact method but IMHO the json parsing should be done outside)

Closes-Bug: #1973243
Change-Id: I24ce4ecd854f8a19ed4d760404adb7d1ac6b5509
This commit is contained in:
Guillaume DeMengin 2022-05-12 23:08:32 +02:00
parent d1e4696495
commit 483b0211bc
2 changed files with 57 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
jenkins/__init__.py
View File

@ -542,13 +542,13 @@ class Jenkins(object):
# when accessing .text property
return response
def _request(self, req):
def _request(self, req, stream=None):
r = self._session.prepare_request(req)
# requests.Session.send() does not honor env settings by design
# see https://github.com/requests/requests/issues/2807
_settings = self._session.merge_environment_settings(
r.url, {}, None, self._session.verify, None)
r.url, {}, stream, self._session.verify, None)
_settings['timeout'] = self.timeout
return self._session.send(r, **_settings)
@ -559,7 +559,14 @@ class Jenkins(object):
'''
return self.jenkins_request(req, add_crumb, resolve_auth).text
def jenkins_request(self, req, add_crumb=True, resolve_auth=True):
def jenkins_open_stream(self, req, add_crumb=True, resolve_auth=True):
'''Return the HTTP response body from a ``requests.Request``.
:returns: ``stream``
'''
return self.jenkins_request(req, add_crumb, resolve_auth, True)
def jenkins_request(self, req, add_crumb=True, resolve_auth=True, stream=None):
'''Utility routine for opening an HTTP request to a Jenkins server.
:param req: A ``requests.Request`` to submit.
@ -567,6 +574,7 @@ class Jenkins(object):
before submitting. Defaults to ``True``.
:param resolve_auth: If True, maybe add authentication. Defaults to
``True``.
:param stream: If True, return a stream
:returns: A ``requests.Response`` object.
'''
try:
@ -576,7 +584,7 @@ class Jenkins(object):
self.maybe_add_crumb(req)
return self._response_handler(
self._request(req))
self._request(req, stream))
except req_exc.HTTPError as e:
# Jenkins's funky authentication means its nigh impossible to
@ -719,23 +727,20 @@ class Jenkins(object):
:param name: Job name, ``str``
:param number: Build number, ``str`` (also accepts ``int``)
:param artifact: Artifact relative path, ``str``
:returns: artifact to download, ``dict``
:returns: artifact to download, ``bytes``
"""
folder_url, short_name = self._get_job_folder(name)
try:
response = self.jenkins_open(requests.Request(
'GET', self._build_url(BUILD_ARTIFACT, locals())))
if response:
return json.loads(response)
else:
raise JenkinsException('job[%s] number[%s] does not exist' % (name, number))
with self.jenkins_open_stream(requests.Request(
'GET', self._build_url(BUILD_ARTIFACT, locals()))) as response:
if response.encoding is None:
return response.raw.read()
else:
return response.text.encode(response.encoding)
raise JenkinsException('job[%s] number[%s] does not exist' % (name, number))
except requests.exceptions.HTTPError:
raise JenkinsException('job[%s] number[%s] does not exist' % (name, number))
except ValueError:
raise JenkinsException(
'Could not parse JSON info for job[%s] number[%s]' % (name, number))
except NotFoundException:
# This can happen if the artifact is not found
return None

61
tests/test_build.py
View File

@ -1,7 +1,7 @@
import json
import collections
from mock import patch
from mock import patch, Mock
import jenkins
from tests.base import JenkinsTestBase
@ -723,32 +723,57 @@ class JenkinsBuildTestReportUrlTest(JenkinsTestBase):
class JenkinsBuildArtifactUrlTest(JenkinsTestBase):
@patch.object(jenkins.Jenkins, 'jenkins_open')
def streamMock(self, encoding=None, text=None, binary=None):
streamMock = Mock()
streamMock.__exit__ = Mock()
streamMock.__enter__ = Mock()
if encoding is None and text is None and binary is None:
streamMock.__enter__.return_value = None
return streamMock
streamMock.__enter__.return_value = Mock()
streamMock.__enter__.return_value.encoding = encoding
streamMock.__enter__.return_value.text = text
streamMock.__enter__.return_value.raw = Mock()
streamMock.__enter__.return_value.raw.read = Mock()
streamMock.__enter__.return_value.raw.read.return_value = binary
return streamMock
@patch.object(jenkins.Jenkins, 'jenkins_open_stream')
def test_simple(self, jenkins_mock):
jenkins_mock.return_value = '{}'
jenkins_mock.return_value = self.streamMock('utf-8', 'ascii')
ret = self.j.get_build_artifact(u'Test Job', number='52', artifact="filename")
self.assertEqual(ret, json.loads(jenkins_mock.return_value))
self.assertEqual(ret, b'ascii')
self.assertEqual(
jenkins_mock.call_args[0][0].url,
self.make_url('job/Test%20Job/52/artifact/filename'))
self._check_requests(jenkins_mock.call_args_list)
@patch.object(jenkins.Jenkins, 'jenkins_open')
@patch.object(jenkins.Jenkins, 'jenkins_open_stream')
def test_simple_binary(self, jenkins_mock):
jenkins_mock.return_value = self.streamMock(binary=b'\0\1\2')
ret = self.j.get_build_artifact(u'Test Job', number='52', artifact="filename")
self.assertEqual(ret, b'\0\1\2')
self.assertEqual(
jenkins_mock.call_args[0][0].url,
self.make_url('job/Test%20Job/52/artifact/filename'))
self._check_requests(jenkins_mock.call_args_list)
@patch.object(jenkins.Jenkins, 'jenkins_open_stream')
def test_in_folder(self, jenkins_mock):
jenkins_mock.return_value = '{}'
jenkins_mock.return_value = self.streamMock('utf-8', 'ascii')
ret = self.j.get_build_artifact(u'a Folder/Test Job', number='52', artifact="file name")
self.assertEqual(ret, json.loads(jenkins_mock.return_value))
self.assertEqual(ret, b'ascii')
self.assertEqual(
jenkins_mock.call_args[0][0].url,
self.make_url('job/a%20Folder/job/Test%20Job/52/artifact/file%20name'))
self._check_requests(jenkins_mock.call_args_list)
@patch.object(jenkins.Jenkins, 'jenkins_open')
@patch.object(jenkins.Jenkins, 'jenkins_open_stream')
def test_matrix(self, jenkins_mock):
jenkins_mock.return_value = '{}'
jenkins_mock.return_value = self.streamMock('utf-8', 'ascii')
ret = self.j.get_build_artifact(u'a Folder/Test Job', number='52/index=matrix',
artifact="file name")
self.assertEqual(ret, json.loads(jenkins_mock.return_value))
self.assertEqual(ret, b'ascii')
self.assertEqual(
jenkins_mock.call_args[0][0].url,
self.make_url('job/a%20Folder/job/Test%20Job/52/index=matrix/artifact/file%20name'))
@ -763,10 +788,9 @@ class JenkinsBuildArtifactUrlTest(JenkinsTestBase):
ret = self.j.get_build_artifact(u'TestJob', number='52', artifact="filename")
self.assertIsNone(ret)
@patch.object(jenkins.Jenkins, 'jenkins_open')
@patch.object(jenkins.Jenkins, 'jenkins_open_stream')
def test_open_return_none(self, jenkins_mock):
jenkins_mock.return_value = None
jenkins_mock.return_value = self.streamMock()
with self.assertRaises(jenkins.JenkinsException) as context_manager:
self.j.get_build_artifact(u'TestJob', number='52', artifact="filename")
self.assertEqual(
@ -774,17 +798,6 @@ class JenkinsBuildArtifactUrlTest(JenkinsTestBase):
'job[TestJob] number[52] does not exist')
self._check_requests(jenkins_mock.call_args_list)
@patch.object(jenkins.Jenkins, 'jenkins_open')
def test_return_invalid_json(self, jenkins_mock):
jenkins_mock.return_value = 'Invalid JSON'
with self.assertRaises(jenkins.JenkinsException) as context_manager:
self.j.get_build_artifact(u'TestJob', number='52', artifact="filename")
self.assertEqual(
str(context_manager.exception),
'Could not parse JSON info for job[TestJob] number[52]')
self._check_requests(jenkins_mock.call_args_list)
@patch('jenkins.requests.Session.send', autospec=True)
def test_raise_HTTPError(self, session_send_mock):
session_send_mock.side_effect = iter([