From 233d63077e5d56a467cffcae4bf9f163fd1973a0 Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Wed, 6 May 2020 10:14:32 -0400 Subject: [PATCH 1/5] Add 'quiet' option for Processor to not print any output Signed-off-by: Christophe Bedard --- .../tracetools_analysis/processor/__init__.py | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/tracetools_analysis/tracetools_analysis/processor/__init__.py b/tracetools_analysis/tracetools_analysis/processor/__init__.py index 9b5a585..f6007ec 100644 --- a/tracetools_analysis/tracetools_analysis/processor/__init__.py +++ b/tracetools_analysis/tracetools_analysis/processor/__init__.py @@ -289,12 +289,14 @@ class Processor(): def __init__( self, *handlers: EventHandler, + quiet: bool = False, **kwargs, ) -> None: """ Create a Processor. :param handlers: the `EventHandler`s to use for processing + :param quiet: whether to not print any output, like progress information :param kwargs: the parameters to pass on to new handlers """ self._initial_handlers = list(handlers) @@ -303,9 +305,10 @@ class Processor(): self._expanded_handlers = self._expand_dependencies(*handlers, **kwargs) self._handler_multimap = self._get_handler_maps(self._expanded_handlers) self._register_with_handlers(self._expanded_handlers) + self._quiet = quiet self._progress_display = ProcessingProgressDisplay( [type(handler).__name__ for handler in self._expanded_handlers], - ) + ) if not self._quiet else None self._processing_done = False @staticmethod @@ -401,12 +404,17 @@ class Processor(): self._check_required_events(events) if not self._processing_done: - self._progress_display.set_work_total(len(events)) - for event in events: - self._process_event(event) - self._progress_display.did_work() + # Split into two versions so that performance is optimal + if self._progress_display is None: + for event in events: + self._process_event(event) + else: + self._progress_display.set_work_total(len(events)) + for event in events: + self._process_event(event) + self._progress_display.did_work() + self._progress_display.done(erase=erase_progress) self._processing_done = True - self._progress_display.done(erase=erase_progress) def _process_event(self, event: DictEvent) -> None: """Process a single event.""" From 9304d1b30eb553903ac7c77e238a6c93ff8d81fd Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Wed, 6 May 2020 20:02:31 -0400 Subject: [PATCH 2/5] Add test for quiet processor option Signed-off-by: Christophe Bedard --- .../test/tracetools_analysis/test_processor.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tracetools_analysis/test/tracetools_analysis/test_processor.py b/tracetools_analysis/test/tracetools_analysis/test_processor.py index e523e15..31e2e10 100644 --- a/tracetools_analysis/test/tracetools_analysis/test_processor.py +++ b/tracetools_analysis/test/tracetools_analysis/test_processor.py @@ -12,6 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +import contextlib +from io import StringIO from typing import Dict from typing import Set import unittest @@ -172,6 +174,21 @@ class TestProcessor(unittest.TestCase): result = processor.get_handler_by_type(StubHandler1) self.assertTrue(result is handler1) + def test_processor_quiet(self) -> None: + handler1 = StubHandler1() + mock_event = { + '_name': 'myeventname', + '_timestamp': 0, + 'cpu_id': 0, + } + temp_stdout = StringIO() + with contextlib.redirect_stdout(temp_stdout): + processor = Processor(handler1, quiet=True) + processor.process([mock_event]) + # Shouldn't be any output + output = temp_stdout.getvalue() + self.assertEqual(0, len(output), f'Processor was not quiet: "{output}"') + if __name__ == '__main__': unittest.main() From 28b61587cf3c19bd620327eb38214267b516b09a Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Sat, 23 May 2020 10:15:15 -0400 Subject: [PATCH 3/5] Add 'quiet' option to loading-related functions Signed-off-by: Christophe Bedard --- .../tracetools_analysis/loading/__init__.py | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/tracetools_analysis/tracetools_analysis/loading/__init__.py b/tracetools_analysis/tracetools_analysis/loading/__init__.py index f6339be..48ff642 100644 --- a/tracetools_analysis/tracetools_analysis/loading/__init__.py +++ b/tracetools_analysis/tracetools_analysis/loading/__init__.py @@ -31,6 +31,7 @@ from ..convert import DEFAULT_CONVERT_FILE_NAME def inspect_input_path( input_path: str, force_conversion: bool = False, + quiet: bool = False, ) -> Tuple[Optional[str], bool]: """ Check input path for a converted file or a trace directory. @@ -43,6 +44,7 @@ def inspect_input_path( :param input_path: the path to a converted file or trace directory :param force_conversion: whether to re-create converted file even if it is found + :param quiet: whether to not print any normal output :return: the path to a converted file (or `None` if could not find), `True` if the given converted file should be (re-)created, `False` otherwise @@ -59,10 +61,14 @@ def inspect_input_path( # Use that as the converted input file converted_file_path = prospective_converted_file if force_conversion: - print(f'found converted file but will re-create it: {prospective_converted_file}') + if not quiet: + print( + f'found converted file but will re-create it: {prospective_converted_file}' + ) return prospective_converted_file, True else: - print(f'found converted file: {prospective_converted_file}') + if not quiet: + print(f'found converted file: {prospective_converted_file}') return prospective_converted_file, False else: # Check if it is a trace directory @@ -81,7 +87,8 @@ def inspect_input_path( converted_file_path = input_path if force_conversion: # It's a file, but re-create it anyway - print(f'found converted file but will re-create it: {converted_file_path}') + if not quiet: + print(f'found converted file but will re-create it: {converted_file_path}') return converted_file_path, True else: # Simplest use-case: given path is an existing converted file @@ -92,15 +99,21 @@ def inspect_input_path( def convert_if_needed( input_path: str, force_conversion: bool = False, + quiet: bool = False, ) -> Optional[str]: """ Inspect input path and convert trace directory to file if necessary. :param input_path: the path to a converted file or trace directory :param force_conversion: whether to re-create converted file even if it is found + :param quiet: whether to not print any output :return: the path to the converted file, or `None` if it failed """ - converted_file_path, create_converted_file = inspect_input_path(input_path, force_conversion) + converted_file_path, create_converted_file = inspect_input_path( + input_path, + force_conversion, + quiet, + ) if converted_file_path is None: return None @@ -118,6 +131,7 @@ def load_file( input_path: str, do_convert_if_needed: bool = True, force_conversion: bool = False, + quiet: bool = False, ) -> List[Dict]: """ Load file containing converted trace events. @@ -125,10 +139,11 @@ def load_file( :param input_path: the path to a converted file or trace directory :param do_convert_if_needed: whether to create the converted file if needed (else, let it fail) :param force_conversion: whether to re-create converted file even if it is found + :param quiet: whether to not print any output :return: the list of events read from the file """ if do_convert_if_needed or force_conversion: - file_path = convert_if_needed(input_path, force_conversion) + file_path = convert_if_needed(input_path, force_conversion, quiet) else: file_path = input_path From 7d3eded902c740253137a33cceadad2607630110 Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Sat, 23 May 2020 10:15:29 -0400 Subject: [PATCH 4/5] Add test for quiet loading option Signed-off-by: Christophe Bedard --- .../test_process_command.py | 29 ++++++++++++++----- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/tracetools_analysis/test/tracetools_analysis/test_process_command.py b/tracetools_analysis/test/tracetools_analysis/test_process_command.py index d076c9f..b3912ce 100644 --- a/tracetools_analysis/test/tracetools_analysis/test_process_command.py +++ b/tracetools_analysis/test/tracetools_analysis/test_process_command.py @@ -1,5 +1,6 @@ #!/usr/bin/env python3 # Copyright 2019 Apex.AI, Inc. +# Copyright 2020 Christophe Bedard # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -13,6 +14,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +import contextlib +from io import StringIO import os import shutil import tempfile @@ -60,31 +63,43 @@ class TestProcessCommand(unittest.TestCase): def tearDown(self): shutil.rmtree(self.test_dir_path) - def test_inspect_input_path(self) -> None: + def test_inspect_input_path( + self, + quiet: bool = False, + ) -> None: # Should find converted file under directory - file_path, create_file = inspect_input_path(self.with_converted_file_dir, False) + file_path, create_file = inspect_input_path(self.with_converted_file_dir, False, quiet) self.assertEqual(self.converted_file_path, file_path) self.assertFalse(create_file) # Should find it but set it to be re-created - file_path, create_file = inspect_input_path(self.with_converted_file_dir, True) + file_path, create_file = inspect_input_path(self.with_converted_file_dir, True, quiet) self.assertEqual(self.converted_file_path, file_path) self.assertTrue(create_file) # Should fail to find converted file under directory - file_path, create_file = inspect_input_path(self.without_converted_file_dir, False) + file_path, create_file = inspect_input_path(self.without_converted_file_dir, False, quiet) self.assertIsNone(file_path) self.assertFalse(create_file) - file_path, create_file = inspect_input_path(self.without_converted_file_dir, True) + file_path, create_file = inspect_input_path(self.without_converted_file_dir, True, quiet) self.assertIsNone(file_path) self.assertFalse(create_file) # Should accept any file path if it exists - file_path, create_file = inspect_input_path(self.random_file_path, False) + file_path, create_file = inspect_input_path(self.random_file_path, False, quiet) self.assertEqual(self.random_file_path, file_path) self.assertFalse(create_file) # Should set it to be re-created - file_path, create_file = inspect_input_path(self.random_file_path, True) + file_path, create_file = inspect_input_path(self.random_file_path, True, quiet) self.assertEqual(self.random_file_path, file_path) self.assertTrue(create_file) # TODO try with a trace directory + + def test_inspect_input_path_quiet(self) -> None: + temp_stdout = StringIO() + with contextlib.redirect_stdout(temp_stdout): + # Just run the other test again + self.test_inspect_input_path(quiet=True) + # Shouldn't be any output + output = temp_stdout.getvalue() + self.assertEqual(0, len(output), f'was not quiet: "{output}"') From bd0cdc67725f101107720b6485905c5194ef1711 Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Sat, 23 May 2020 10:16:38 -0400 Subject: [PATCH 5/5] Rename TestProcessCommand to TestLoading Signed-off-by: Christophe Bedard --- .../{test_process_command.py => test_loading.py} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename tracetools_analysis/test/tracetools_analysis/{test_process_command.py => test_loading.py} (98%) diff --git a/tracetools_analysis/test/tracetools_analysis/test_process_command.py b/tracetools_analysis/test/tracetools_analysis/test_loading.py similarity index 98% rename from tracetools_analysis/test/tracetools_analysis/test_process_command.py rename to tracetools_analysis/test/tracetools_analysis/test_loading.py index b3912ce..f60a82f 100644 --- a/tracetools_analysis/test/tracetools_analysis/test_process_command.py +++ b/tracetools_analysis/test/tracetools_analysis/test_loading.py @@ -24,7 +24,7 @@ import unittest from tracetools_analysis.loading import inspect_input_path -class TestProcessCommand(unittest.TestCase): +class TestLoading(unittest.TestCase): def __init__(self, *args) -> None: super().__init__(