Skip to content

Commit

Permalink
Ensure we use application/octet-stream for file upload.
Browse files Browse the repository at this point in the history
Nexus, at the minimum, will fail to process the uploaded file if we
omit this header (I assume it considers the incoming body as text vs
a stream of bytes).

Nexus also doesn't return a body on succesful create. This caused
the upload_result to be empty, which made the artifact value empty,
which caused a failure. Always create a valid return value
that describes the HTTP status code, the reason, and any body.
  • Loading branch information
adamgoossens authored and itewk committed Jun 22, 2021
1 parent 32cd907 commit d6a75ad
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 5 deletions.
10 changes: 8 additions & 2 deletions src/ploigos_step_runner/utils/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -267,11 +267,17 @@ def upload_file(file_path, destination_uri, username=None, password=None): # pyl
opener = urllib.request.build_opener()

with open(file_path, 'rb') as file:
request = urllib.request.Request(url=destination_uri, data=file.read(), method='PUT')
request = urllib.request.Request(url=destination_uri, data=file.read(), method='PUT',
headers={'Content-Type': 'application/octet-stream'})

try:
result = opener.open(request)
upload_result = str(result.read(), encoding='utf8')
response_reason = result.reason
response_body = str(result.read(), encoding='utf8')
response_code = result.status
upload_result = f"status={response_code}, " \
f"reason={response_reason}, " \
f"body={response_body}"
except urllib.error.HTTPError as error:
raise RuntimeError(
f"Error uploading file ({file_path}) to destination ({destination_uri})"
Expand Down
8 changes: 5 additions & 3 deletions tests/utils/test_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,8 @@ def __create_http_response_side_effect(self, read_return):
def http_response_side_effect(request):
mock_response = Mock()
mock_response.read.return_value = read_return
mock_response.status = '201'
mock_response.reason = 'Created'
return mock_response

return http_response_side_effect
Expand Down Expand Up @@ -278,7 +280,7 @@ def test_http_prefix_no_auth(self, opener_open_mock):
file_path=sample_file_path,
destination_uri="http://ploigos.com/test/foo42"
)
self.assertEqual('hello world 42', actual_result)
self.assertEqual('status=201, reason=Created, body=hello world 42', actual_result)

@patch.object(
urllib.request.OpenerDirector,
Expand All @@ -297,7 +299,7 @@ def test_https_prefix_no_auth(self, opener_open_mock):
file_path=sample_file_path,
destination_uri="https://ploigos.com/test/foo42"
)
self.assertEqual('hello world 42', actual_result)
self.assertEqual('status=201, reason=Created, body=hello world 42', actual_result)

@patch.object(
urllib.request.OpenerDirector,
Expand All @@ -319,7 +321,7 @@ def test_https_prefix_auth(self, pass_manager_mock, opener_open_mock):
username='test_user',
password='pass123'
)
self.assertEqual('hello world 42', actual_result)
self.assertEqual('status=201, reason=Created, body=hello world 42', actual_result)

pass_manager_mock().add_password.assert_called_once_with(
None,
Expand Down

0 comments on commit d6a75ad

Please sign in to comment.