Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add no progress result printer to s3 transfer #2747

Merged
merged 5 commits into from
Oct 13, 2017
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions awscli/customizations/s3/results.py
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,12 @@ def _clear_progress_if_no_more_expected_transfers(self, **kwargs):
uni_print(self._adjust_statement_padding(''), self._out_file)


class NoProgressResultPrinter(ResultPrinter):
"""A result printer that doesn't print progress"""
def _print_progress(self, **kwargs):
pass


class OnlyShowErrorsResultPrinter(ResultPrinter):
"""A result printer that only prints out errors"""
def _print_progress(self, **kwargs):
Expand Down
3 changes: 3 additions & 0 deletions awscli/customizations/s3/s3handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
from awscli.customizations.s3.results import ResultRecorder
from awscli.customizations.s3.results import ResultPrinter
from awscli.customizations.s3.results import OnlyShowErrorsResultPrinter
from awscli.customizations.s3.results import NoProgressResultPrinter
from awscli.customizations.s3.results import ResultProcessor
from awscli.customizations.s3.results import CommandResultRecorder
from awscli.customizations.s3.utils import RequestParamsMapper
Expand Down Expand Up @@ -110,6 +111,8 @@ def _add_result_printer(self, result_recorder, result_processor_handlers):
result_printer = OnlyShowErrorsResultPrinter(result_recorder)
elif self._cli_params.get('is_stream'):
result_printer = OnlyShowErrorsResultPrinter(result_recorder)
elif self._cli_params.get('no_progress'):
result_printer = NoProgressResultPrinter(result_recorder)
else:
result_printer = ResultPrinter(result_recorder)
result_processor_handlers.append(result_printer)
Expand Down
7 changes: 6 additions & 1 deletion awscli/customizations/s3/subcommands.py
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,11 @@
'output is suppressed.')}


NO_PROGRESS = {'name': 'no-progress', 'action': 'store_true',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The no-* params typically have a store_false action with the destination being the param without the no prefix (progress in this case). Also makes it less confusing if we ever add an explicit true version of this arg (--progress).

'help_text': (
'File transfer progress is not displayed.')}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably worth documenting that this arg only matter is the other two options aren't provided (quiet/only_show_errors)



EXPECTED_SIZE = {'name': 'expected-size',
'help_text': (
'This argument specifies the expected size of a stream '
Expand Down Expand Up @@ -424,7 +429,7 @@
SSE_C_COPY_SOURCE_KEY, STORAGE_CLASS, GRANTS,
WEBSITE_REDIRECT, CONTENT_TYPE, CACHE_CONTROL,
CONTENT_DISPOSITION, CONTENT_ENCODING, CONTENT_LANGUAGE,
EXPIRES, SOURCE_REGION, ONLY_SHOW_ERRORS,
EXPIRES, SOURCE_REGION, ONLY_SHOW_ERRORS, NO_PROGRESS,
PAGE_SIZE, IGNORE_GLACIER_WARNINGS, FORCE_GLACIER_TRANSFER]


Expand Down
11 changes: 11 additions & 0 deletions tests/integration/customizations/s3/test_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -1255,6 +1255,17 @@ def test_normal_output_only_show_errors(self):
# Check that nothing was printed to stdout.
self.assertEqual('', p.stdout)

def test_normal_output_no_progress(self):
bucket_name = _SHARED_BUCKET
foo_txt = self.files.create_file('foo.txt', 'foo contents')

# Copy file into bucket.
p = aws('s3 cp %s s3://%s/ --no-progress' % (foo_txt, bucket_name))
self.assertEqual(p.rc, 0)
# Ensure success message was printed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be good to assert there was no progress statement as well in the standard output.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

self.assertIn('upload', p.stdout)
self.assertIn('s3://%s/foo.txt' % bucket_name, p.stdout)

def test_error_output(self):
foo_txt = self.files.create_file('foo.txt', 'foo contents')

Expand Down
68 changes: 68 additions & 0 deletions tests/unit/customizations/s3/test_results.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
from awscli.customizations.s3.results import ResultRecorder
from awscli.customizations.s3.results import ResultPrinter
from awscli.customizations.s3.results import OnlyShowErrorsResultPrinter
from awscli.customizations.s3.results import NoProgressResultPrinter
from awscli.customizations.s3.results import ResultProcessor
from awscli.customizations.s3.results import CommandResultRecorder
from awscli.customizations.s3.utils import relative_path
Expand Down Expand Up @@ -1517,6 +1518,73 @@ def test_print_unicode_error(self):
self.assertEqual(self.error_file.getvalue(), ref_error_statement)


class TestNoProgressResultPrinter(BaseResultPrinterTest):
def setUp(self):
super(TestNoProgressResultPrinter, self).setUp()
self.result_printer = NoProgressResultPrinter(
result_recorder=self.result_recorder,
out_file=self.out_file,
error_file=self.error_file
)

def test_does_not_print_progress_result(self):
progress_result = self.get_progress_result()
self.result_printer(progress_result)
self.assertEqual(self.out_file.getvalue(), '')

def test_does_print_sucess_result(self):
transfer_type = 'upload'
src = 'file'
dest = 's3://mybucket/mykey'
success_result = SuccessResult(
transfer_type=transfer_type, src=src, dest=dest)

self.result_printer(success_result)
expected_message = 'upload: file to s3://mybucket/mykey\n'
self.assertEqual(self.out_file.getvalue(), expected_message)

def test_print_failure_result(self):
transfer_type = 'upload'
src = 'file'
dest = 's3://mybucket/mykey'
failure_result = FailureResult(
transfer_type=transfer_type, src=src, dest=dest,
exception=Exception('my exception'))

self.result_printer(failure_result)

ref_failure_statement = (
'upload failed: file to s3://mybucket/mykey my exception\n'
)
self.assertEqual(self.error_file.getvalue(), ref_failure_statement)

def test_print_warnings_result(self):
self.result_printer(WarningResult('warning: my warning'))
ref_warning_statement = 'warning: my warning\n'
self.assertEqual(self.error_file.getvalue(), ref_warning_statement)

def test_final_total_does_not_try_to_clear_empty_progress(self):
transfer_type = 'upload'
src = 'file'
dest = 's3://mybucket/mykey'

mb = 1024 * 1024
self.result_recorder.expected_files_transferred = 1
self.result_recorder.files_transferred = 1
self.result_recorder.expected_bytes_transferred = mb
self.result_recorder.bytes_transferred = mb

success_result = SuccessResult(
transfer_type=transfer_type, src=src, dest=dest)
self.result_printer(success_result)
ref_statement = 'upload: file to s3://mybucket/mykey\n'
self.assertEqual(self.out_file.getvalue(), ref_statement)

self.result_recorder.final_expected_files_transferred = 1
self.result_printer(FinalTotalSubmissionsResult(1))
self.assertEqual(self.out_file.getvalue(), ref_statement)


class TestOnlyShowErrorsResultPrinter(BaseResultPrinterTest):
def setUp(self):
super(TestOnlyShowErrorsResultPrinter, self).setUp()
Expand Down