Skip to content

Commit

Permalink
Print failure when create local file fails
Browse files Browse the repository at this point in the history
Fix #1645

This only applies when downloading large files. See issue #1645 for more
details
  • Loading branch information
phss committed Nov 16, 2015
1 parent b9984c9 commit b31d36d
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 4 deletions.
5 changes: 3 additions & 2 deletions awscli/customizations/s3/s3handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,9 @@ def _enqueue_range_download_tasks(self, filename, remove_remote_file=False):
chunksize = find_chunksize(filename.size, self.chunksize)
num_downloads = int(filename.size / chunksize)
context = tasks.MultipartDownloadContext(num_downloads)
create_file_task = tasks.CreateLocalFileTask(context=context,
filename=filename)
create_file_task = tasks.CreateLocalFileTask(
context=context, filename=filename,
result_queue=self.result_queue)
self.executor.submit(create_file_task)
self._do_enqueue_range_download_tasks(
filename=filename, chunksize=chunksize,
Expand Down
8 changes: 7 additions & 1 deletion awscli/customizations/s3/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -263,9 +263,10 @@ def __call__(self):


class CreateLocalFileTask(OrderableTask):
def __init__(self, context, filename):
def __init__(self, context, filename, result_queue):
self._context = context
self._filename = filename
self._result_queue = result_queue

def __call__(self):
dirname = os.path.dirname(self._filename.dest)
Expand All @@ -284,6 +285,11 @@ def __call__(self):
with open(self._filename.dest, 'wb'):
pass
except Exception as e:
message = print_operation(self._filename, failed=True,
dryrun=False)
message += '\n' + str(e)
result = {'message': message, 'error': True}
self._result_queue.put(PrintTask(**result))
self._context.cancel()
else:
self._context.announce_file_created()
Expand Down
39 changes: 38 additions & 1 deletion tests/unit/customizations/s3/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
import threading
import mock
import socket
import os
import tempfile
import shutil
from six.moves import queue

from botocore.exceptions import IncompleteReadError
from botocore.vendored.requests.packages.urllib3.exceptions import \
Expand Down Expand Up @@ -326,6 +330,39 @@ def test_print_operation(self):
self.assertIn(r'e:\foo', message)


class TestCreateLocalFileTask(unittest.TestCase):
def setUp(self):
self.result_queue = queue.Queue()
self.tempdir = tempfile.mkdtemp()
self.filename = mock.Mock()
self.filename.src = 'bucket/key'
self.filename.dest = os.path.join(self.tempdir, 'local', 'file')
self.filename.operation_name = 'download'
self.context = mock.Mock()
self.task = CreateLocalFileTask(self.context,
self.filename,
self.result_queue)

def tearDown(self):
shutil.rmtree(self.tempdir)

def test_creates_file_and_announces(self):
self.task()
self.assertTrue(os.path.isfile(self.filename.dest))
self.context.announce_file_created.assert_called_with()
self.assertTrue(self.result_queue.empty())

def test_cancel_command_on_exception(self):
# Set destination directory to read-only
os.chmod(self.tempdir, 444)
self.task()
self.assertFalse(os.path.isfile(self.filename.dest))
self.context.cancel.assert_called_with()
self.assertFalse(self.result_queue.empty())
error_message = self.result_queue.get()
self.assertIn("download failed", error_message.message)


class TestDownloadPartTask(unittest.TestCase):
def setUp(self):
self.result_queue = mock.Mock()
Expand Down Expand Up @@ -495,7 +532,7 @@ def setUp(self):
def create_task(self):
# We don't actually care about the arguments, we just want to test
# the ordering of the tasks.
return CreateLocalFileTask(None, None)
return CreateLocalFileTask(None, None, None)

def complete_task(self):
return CompleteDownloadTask(None, None, None, None, None)
Expand Down

0 comments on commit b31d36d

Please sign in to comment.