From c0bbc66878596a9acc6ae0ec9ea8d14fb996faf9 Mon Sep 17 00:00:00 2001 From: Guillaume DeMengin Date: Thu, 12 May 2022 23:08:32 +0200 Subject: [PATCH] have get_build_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 --- jenkins/__init__.py | 35 +++++++++++++++----------- tests/test_build.py | 61 +++++++++++++++++++++++++++------------------ 2 files changed, 57 insertions(+), 39 deletions(-) diff --git a/jenkins/__init__.py b/jenkins/__init__.py index 7ebdbea..1b56150 100755 --- a/jenkins/__init__.py +++ b/jenkins/__init__.py @@ -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 diff --git a/tests/test_build.py b/tests/test_build.py index acd3a58..c833c43 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -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([