Skip to content

Commit

Permalink
Errors and warnings are printed to stderr.
Browse files Browse the repository at this point in the history
  • Loading branch information
kyleknap committed Sep 23, 2014
1 parent 0f0000a commit 8ad1152
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 34 deletions.
14 changes: 11 additions & 3 deletions awscli/customizations/s3/executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -274,13 +274,15 @@ def run(self):

def _process_print_task(self, print_task):
print_str = print_task.message
print_to_stderr = False
if print_task.error:
self.num_errors_seen += 1
print_to_stderr = True
warning = False
if print_task.warning:
if print_task.warning:
warning = True
self.num_warnings_seen += 1
warning = True
self.num_warnings_seen += 1
print_to_stderr = True
final_str = ''
if warning:
final_str += print_str.ljust(self._progress_length, ' ')
Expand Down Expand Up @@ -309,6 +311,12 @@ def _process_print_task(self, print_task):
self._num_parts += 1
self._file_count += 1

# If the message is an error or warning, print it to standard error.
if print_to_stderr == True and not self._quiet:
uni_print(final_str, sys.stderr)
sys.stderr.flush()
final_str = ''

is_done = self._total_files == self._file_count
if not is_done:
prog_str = "Completed %s " % self._num_parts
Expand Down
20 changes: 11 additions & 9 deletions awscli/customizations/s3/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -223,24 +223,26 @@ def __init__(self):
self.count = 0


def uni_print(statement):
def uni_print(statement, out_file=None):
"""
This function is used to properly write unicode to stdout. It
ensures that the proper encoding is used if the statement is
not in a version type of string. The initial check is to
allow if ``sys.stdout`` does not use an encoding
This function is used to properly write unicode to a file, usually
stdout or stdderr. It ensures that the proper encoding is used if the
statement is not a string type. The initial check is to allow if
``out_file`` does not use an encoding.
"""
encoding = getattr(sys.stdout, 'encoding', None)
if out_file is None:
out_file = sys.stdout
encoding = getattr(out_file, 'encoding', None)
if encoding is not None and not PY3:
sys.stdout.write(statement.encode(sys.stdout.encoding))
out_file.write(statement.encode(out_file.encoding))
else:
try:
sys.stdout.write(statement)
out_file.write(statement)
except UnicodeEncodeError:
# Some file like objects like cStringIO will
# try to decode as ascii. Interestingly enough
# this works with a normal StringIO.
sys.stdout.write(statement.encode('utf-8'))
out_file.write(statement.encode('utf-8'))


def guess_content_type(filename):
Expand Down
22 changes: 11 additions & 11 deletions tests/integration/customizations/s3/test_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ def test_download_non_existent_key(self):
expected_err_msg = (
'A client error (NoSuchKey) occurred when calling the '
'HeadObject operation: Key "foo.txt" does not exist')
self.assertIn(expected_err_msg, p.stdout)
self.assertIn(expected_err_msg, p.stderr)


class TestSync(BaseS3CLICommand):
Expand Down Expand Up @@ -645,7 +645,7 @@ def testFailWithoutRegion(self):
p2 = aws('s3 sync s3://%s/ s3://%s/ --region %s' %
(self.src_bucket, self.dest_bucket, self.src_region))
self.assertEqual(p2.rc, 1, p2.stdout)
self.assertIn('PermanentRedirect', p2.stdout)
self.assertIn('PermanentRedirect', p2.stderr)

def testCpRegion(self):
self.files.create_file('foo.txt', 'foo')
Expand Down Expand Up @@ -695,9 +695,9 @@ def extra_setup(self):
def test_no_exist(self):
filename = os.path.join(self.files.rootdir, "no-exists-file")
p = aws('s3 cp %s s3://%s/' % (filename, self.bucket_name))
self.assertEqual(p.rc, 2, p.stdout)
self.assertEqual(p.rc, 2, p.stderr)
self.assertIn('warning: Skipping file %s. File does not exist.' %
filename, p.stdout)
filename, p.stderr)

@unittest.skipIf(platform.system() not in ['Darwin', 'Linux'],
'Read permissions tests only supported on mac/linux')
Expand All @@ -711,9 +711,9 @@ def test_no_read_access(self):
permissions = permissions ^ stat.S_IREAD
os.chmod(filename, permissions)
p = aws('s3 cp %s s3://%s/' % (filename, self.bucket_name))
self.assertEqual(p.rc, 2, p.stdout)
self.assertEqual(p.rc, 2, p.stderr)
self.assertIn('warning: Skipping file %s. File/Directory is '
'not readable.' % filename, p.stdout)
'not readable.' % filename, p.stderr)

@unittest.skipIf(platform.system() not in ['Darwin', 'Linux'],
'Special files only supported on mac/linux')
Expand All @@ -723,10 +723,10 @@ def test_is_special_file(self):
sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
sock.bind(file_path)
p = aws('s3 cp %s s3://%s/' % (file_path, self.bucket_name))
self.assertEqual(p.rc, 2, p.stdout)
self.assertEqual(p.rc, 2, p.stderr)
self.assertIn(("warning: Skipping file %s. File is character "
"special device, block special device, FIFO, or "
"socket." % file_path), p.stdout)
"socket." % file_path), p.stderr)


@unittest.skipIf(platform.system() not in ['Darwin', 'Linux'],
Expand Down Expand Up @@ -806,10 +806,10 @@ def test_follow_symlinks_default(self):

def test_bad_symlink(self):
p = aws('s3 sync %s s3://%s/' % (self.files.rootdir, self.bucket_name))
self.assertEqual(p.rc, 2, p.stdout)
self.assertEqual(p.rc, 2, p.stderr)
self.assertIn('warning: Skipping file %s. File does not exist.' %
os.path.join(self.files.rootdir, 'b-badsymlink'),
p.stdout)
p.stderr)


class TestUnicode(BaseS3CLICommand):
Expand Down Expand Up @@ -945,7 +945,7 @@ def test_mb_rb(self):
def test_fail_mb_rb(self):
# Choose a bucket name that already exists.
p = aws('s3 mb s3://mybucket')
self.assertIn("BucketAlreadyExists", p.stdout)
self.assertIn("BucketAlreadyExists", p.stderr)
self.assertEqual(p.rc, 1)


Expand Down
8 changes: 4 additions & 4 deletions tests/integration/customizations/s3/test_s3handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,12 +163,12 @@ def setUp(self):
self.s3_files = [self.bucket + '/text1.txt',
self.bucket + '/another_directory/text2.txt']
self.output = StringIO()
self.saved_stdout = sys.stdout
sys.stdout = self.output
self.saved_stderr = sys.stderr
sys.stderr = self.output

def tearDown(self):
self.output.close()
sys.stdout = self.saved_stdout
sys.stderr = self.saved_stderr
clean_loc_files(self.loc_files)
s3_cleanup(self.bucket, self.session)

Expand Down Expand Up @@ -215,7 +215,7 @@ def setUp(self):
self.session = botocore.session.get_session(EnvironmentVariables)
self.service = self.session.get_service('s3')
self.endpoint = self.service.get_endpoint('us-east-1')
params = {'region': 'us-east-1', 'acl': ['private']}
params = {'region': 'us-east-1', 'acl': ['private'], 'quiet': True}
self.s3_handler = S3Handler(self.session, params)
self.bucket = make_s3_files(self.session, key1=u'\u2713')
self.bucket2 = create_bucket(self.session)
Expand Down
14 changes: 11 additions & 3 deletions tests/unit/customizations/s3/test_executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,20 @@ def __call__(self):
executor.wait_until_shutdown()
self.assertEqual(open(f.name, 'rb').read(), b'foobar')


class TestPrintThread(unittest.TestCase):
def test_print_error(self):
result_queue = queue.Queue()
print_task = PrintTask(message="Fail File.", error=True)
thread = PrintThread(result_queue, False)
with mock.patch('sys.stderr', new=six.StringIO()) as mock_stderr:
thread._process_print_task(print_task)
self.assertIn("Fail File.", mock_stderr.getvalue())

def test_print_warning(self):
result_queue = queue.Queue()
print_task = PrintTask(message="Bad File.", warning=True)
thread = PrintThread(result_queue, False)
with mock.patch('sys.stdout', new=six.StringIO()) as mock_stdout:
with mock.patch('sys.stderr', new=six.StringIO()) as mock_stderr:
thread._process_print_task(print_task)
self.assertIn("Bad File.", mock_stdout.getvalue())

self.assertIn("Bad File.", mock_stderr.getvalue())
4 changes: 2 additions & 2 deletions tests/unit/customizations/s3/test_s3handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ def setUp(self):
self.session = FakeSession()
self.service = self.session.get_service('s3')
self.endpoint = self.service.get_endpoint('us-east-1')
params = {'region': 'us-east-1', 'acl': ['private']}
params = {'region': 'us-east-1', 'acl': ['private'], 'quiet': True}
self.s3_handler = S3Handler(self.session, params)
self.s3_handler_multi = S3Handler(self.session, multi_threshold=10,
chunksize=2,
Expand Down Expand Up @@ -275,7 +275,7 @@ def setUp(self):
self.session = FakeSession(True, True)
self.service = self.session.get_service('s3')
self.endpoint = self.service.get_endpoint('us-east-1')
params = {'region': 'us-east-1'}
params = {'region': 'us-east-1', 'quiet': True}
self.s3_handler_multi = S3Handler(self.session, params,
multi_threshold=10, chunksize=2)
self.bucket = create_bucket(self.session)
Expand Down
9 changes: 7 additions & 2 deletions tests/unit/customizations/s3/test_subcommands.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,17 @@ def setUp(self):
self.bucket = make_s3_files(self.session)
self.loc_files = make_loc_files()
self.output = StringIO()
self.err_output = StringIO()
self.saved_stdout = sys.stdout
self.saved_stderr = sys.stderr
sys.stdout = self.output
sys.stderr = self.err_output

def tearDown(self):
self.output.close()
self.err_output.close()
sys.stdout = self.saved_stdout
sys.stderr = self.saved_stderr

super(CommandArchitectureTest, self).tearDown()
clean_loc_files(self.loc_files)
Expand Down Expand Up @@ -222,7 +227,7 @@ def test_error_on_same_line_as_status(self):
output_str = (
"upload failed: %s to %s Error: Bucket does not exist\n" % (
rel_local_file, s3_file))
self.assertIn(output_str, self.output.getvalue())
self.assertIn(output_str, self.err_output.getvalue())

def test_run_cp_get(self):
# This ensures that the architecture sets up correctly for a ``cp`` get
Expand Down Expand Up @@ -362,7 +367,7 @@ def test_run_rb_nonzero_rc(self):
cmd_arc.create_instructions()
rc = cmd_arc.run()
output_str = "remove_bucket failed: %s" % s3_prefix
self.assertIn(output_str, self.output.getvalue())
self.assertIn(output_str, self.err_output.getvalue())
self.assertEqual(rc, 1)


Expand Down

0 comments on commit 8ad1152

Please sign in to comment.