From 3fb044e4e6f9194bd24779035c70ab4f71209a3e Mon Sep 17 00:00:00 2001 From: Guillaume Fieni Date: Thu, 28 May 2026 17:00:57 +0200 Subject: [PATCH] feat(cli): Support comma-separated list arguments --- src/powerapi/cli/_utils.py | 20 ++++++++++++++++--- .../cli/common_cli_parsing_manager.py | 8 ++++---- src/powerapi/cli/config_parser.py | 19 ++++++------------ src/powerapi/cli/config_validator.py | 10 ++-------- src/powerapi/cli/generator.py | 11 +--------- src/powerapi/cli/parsing_manager.py | 2 +- tests/unit/cli/conftest.py | 8 +++----- tests/unit/cli/test_config_validator.py | 13 ++---------- tests/unit/cli/test_generator.py | 4 ---- tests/unit/cli/test_generator_influxdb2.py | 1 - tests/unit/cli/test_generator_prometheus.py | 2 +- ...put_stream_mode_enabled_configuration.json | 2 +- ...uts_stream_mode_enabled_configuration.json | 2 +- 13 files changed, 39 insertions(+), 63 deletions(-) diff --git a/src/powerapi/cli/_utils.py b/src/powerapi/cli/_utils.py index 8f6a1265..20d528eb 100644 --- a/src/powerapi/cli/_utils.py +++ b/src/powerapi/cli/_utils.py @@ -36,11 +36,25 @@ def find_longest_string_in_list(strings: list[str]) -> str: return max(strings, key=len) -def string_to_bool(bool_value: str): +def string_to_bool(value: str) -> bool: """ - Transforms a str to bool according to their content + Transforms a string to a boolean according to its content. + :param value: The string to be converted + :return: Boolean value """ - return bool_value.lower() in ("yes", "true", "t", "1") + return value.casefold() in ("yes", "y", "true", "t", "1") + + +def string_to_list(value: str) -> list: + """ + Transforms a comma separated list to a list of strings. + :param value: The string to be converted + :return: List of strings + """ + if value == '': + return [] + + return [v.strip() for v in value.split(',')] def merge_dictionaries(source: dict, destination: dict) -> dict: diff --git a/src/powerapi/cli/common_cli_parsing_manager.py b/src/powerapi/cli/common_cli_parsing_manager.py index 6006e1ae..b792265c 100644 --- a/src/powerapi/cli/common_cli_parsing_manager.py +++ b/src/powerapi/cli/common_cli_parsing_manager.py @@ -30,7 +30,7 @@ import sys from powerapi.cli.parsing_manager import RootConfigParsingManager, SubgroupConfigParsingManager -from powerapi.cli.config_parser import store_true, extract_file_names +from powerapi.cli.config_parser import store_true from powerapi.cli.config_parser import MissingValueException from powerapi.exception import BadTypeException, BadContextException, UnknownArgException @@ -138,7 +138,7 @@ def __init__(self): "f", "files", help_text="specify input csv files with this format : file1,file2,file3", - action=extract_file_names, + argument_type=list, is_mandatory=True ) subparser_csv_input.add_argument( @@ -219,7 +219,8 @@ def __init__(self): ) subparser_prometheus_output.add_argument( "t", "tags", - help_text="List of metadata tags that will be exposed with the metrics" + help_text="List of metadata tags that will be exposed with the metrics", + argument_type=list ) self.add_subgroup_parser( subgroup_name="output", @@ -240,7 +241,6 @@ def __init__(self): default_value="PowerReport", ) - subparser_csv_output.add_argument("t", "tags", help_text="List of tags that should be kept") subparser_csv_output.add_argument( "n", "name", help_text="specify pusher name", default_value="pusher_csv" ) diff --git a/src/powerapi/cli/config_parser.py b/src/powerapi/cli/config_parser.py index a8f54bd4..8cd976a1 100644 --- a/src/powerapi/cli/config_parser.py +++ b/src/powerapi/cli/config_parser.py @@ -40,13 +40,12 @@ MissingArgumentException, SameLengthArgumentNamesException, InvalidPrefixException, RepeatedArgumentException, \ SubgroupDoesNotExistException, AlreadyAddedSubgroupException from ._utils import find_longest_string_in_list, string_to_bool, to_lower_case_and_replace_separators, \ - get_longest_related_suffix + get_longest_related_suffix, string_to_list def store_val(argument_name: str, val: Any, configuration: dict, args: list | None = None) -> (list, dict): """ Action that stores the value of the argument on the parser result - """ if val == '': val = None @@ -58,18 +57,8 @@ def store_val(argument_name: str, val: Any, configuration: dict, args: list | No def store_true(argument_name: str, configuration: dict, val: Any = None, args: list | None = None) -> (list, dict): """ Action that stores a True boolean value on the parser result - - """ - val = True - configuration[argument_name] = val - return args, configuration - - -def extract_file_names(argument_name: str, val: Any, configuration: dict, args: list | None = None): - """ - Action used to convert string from --files parameter into a list of file name """ - configuration[argument_name] = val.split(",") + configuration[argument_name] = True return args, configuration @@ -780,6 +769,10 @@ def cast_argument_value(arg_name: str, val: Any, argument: ConfigurationArgument try: if argument.type is bool and val is not None and isinstance(val, str): return string_to_bool(val) + + if argument.type is list and val is not None and isinstance(val, str): + return string_to_list(val) + return argument.type(val) except ValueError as exn: raise BadTypeException(arg_name, argument.type) from exn diff --git a/src/powerapi/cli/config_validator.py b/src/powerapi/cli/config_validator.py index 64ab147e..dfdd0869 100644 --- a/src/powerapi/cli/config_validator.py +++ b/src/powerapi/cli/config_validator.py @@ -103,15 +103,9 @@ def _validate_input(config: dict): """ Check that csv input type has files that exist """ - for key, input_config in config['input'].items(): + for input_config in config['input'].values(): if input_config['type'] == 'csv': - list_of_files = input_config['files'] - - if isinstance(list_of_files, str): - list_of_files = input_config['files'].split(",") - config['input'][key]['files'] = list_of_files - - for file_name in list_of_files: + for file_name in input_config['files']: if not os.access(file_name, os.R_OK): raise FileDoesNotExistException(file_name=file_name) diff --git a/src/powerapi/cli/generator.py b/src/powerapi/cli/generator.py index d7a79fcc..bdd59426 100644 --- a/src/powerapi/cli/generator.py +++ b/src/powerapi/cli/generator.py @@ -107,15 +107,6 @@ def _gen_actor(self, component_config: dict, main_config: dict, component_name: raise NotImplementedError() -def gen_tag_list(db_config: dict): - """ - Generate tag list from tag string - """ - if 'tags' not in db_config or not db_config['tags']: - return [] - return db_config['tags'].split(',') - - class BaseGenerator(Generator): """ Generate an Actor and Start message from config @@ -325,7 +316,7 @@ def _prometheus_database_factory(conf: dict) -> WritableDatabase: Prometheus database factory method. """ from powerapi.database.prometheus import Prometheus - return Prometheus(conf['model'], conf['addr'], conf['port'], gen_tag_list(conf)) + return Prometheus(conf['model'], conf['addr'], conf['port'], conf.get('tags', [])) def __init__(self): super().__init__('output') diff --git a/src/powerapi/cli/parsing_manager.py b/src/powerapi/cli/parsing_manager.py index bbf3389e..c2a70acd 100644 --- a/src/powerapi/cli/parsing_manager.py +++ b/src/powerapi/cli/parsing_manager.py @@ -88,7 +88,7 @@ def validate(self, conf: dict) -> dict: for _, waited_value in self.cli_parser.get_arguments().items(): if args in waited_value.names: # check type - if not isinstance(value, waited_value.type) and not waited_value.is_flag and args != 'files': + if not isinstance(value, waited_value.type) and not waited_value.is_flag: raise BadTypeException(args, waited_value.type) # Check that all the mandatory arguments are present diff --git a/tests/unit/cli/conftest.py b/tests/unit/cli/conftest.py index 43a946b7..92bd7be7 100644 --- a/tests/unit/cli/conftest.py +++ b/tests/unit/cli/conftest.py @@ -143,9 +143,8 @@ def create_empty_files_from_config(invalid_csv_io_stream_config: dict): """ for _, input_config in invalid_csv_io_stream_config['input'].items(): if input_config['type'] == 'csv': - list_of_files = input_config['files'].split(",") - for file_str in list_of_files: - if os.path.isfile(file_str) is False: + for file_str in input_config['files']: + if not os.path.isfile(file_str): with open(file_str, 'w') as file: file.close() @@ -153,8 +152,7 @@ def create_empty_files_from_config(invalid_csv_io_stream_config: dict): for _, input_config in invalid_csv_io_stream_config['input'].items(): if input_config['type'] == 'csv': - list_of_files = input_config['files'] - for file_str in list_of_files: + for file_str in input_config['files']: if os.path.isfile(file_str): os.remove(file_str) diff --git a/tests/unit/cli/test_config_validator.py b/tests/unit/cli/test_config_validator.py index 67ec8181..b169e0ac 100644 --- a/tests/unit/cli/test_config_validator.py +++ b/tests/unit/cli/test_config_validator.py @@ -49,17 +49,10 @@ def test_config_in_postmortem_mode_with_csv_input_is_validated(create_empty_file The files list for the input has to be transformed into a list """ try: - expected_result = load_configuration_from_json_file( - file_name='csv_input_output_stream_mode_enabled_configuration.json') - for current_input in expected_result['input']: - if expected_result['input'][current_input]['type'] == 'csv': - expected_result['input'][current_input]['files'] = ( - expected_result['input'][current_input]['files']).split(',') - + expected_result = load_configuration_from_json_file('csv_input_output_stream_mode_enabled_configuration.json') expected_result['stream'] = False ConfigValidator.validate(csv_io_postmortem_config) - assert csv_io_postmortem_config == expected_result except NotAllowedArgumentValueException as e: @@ -74,11 +67,9 @@ def test_valid_config_postmortem_csv_input_without_optional_arguments_is_validat """ expected_result = csv_io_postmortem_config_without_optional_arguments.copy() for current_input in expected_result['input']: - if expected_result['input'][current_input]['type'] == 'csv': - expected_result['input'][current_input]['files'] = (expected_result['input'][current_input]['files']).split( - ',') expected_result['input'][current_input]['name'] = 'default_puller' expected_result['input'][current_input]['model'] = 'HWPCReport' + expected_result['stream'] = False expected_result['verbose'] = False diff --git a/tests/unit/cli/test_generator.py b/tests/unit/cli/test_generator.py index 28ae9b45..0e63aa6c 100644 --- a/tests/unit/cli/test_generator.py +++ b/tests/unit/cli/test_generator.py @@ -56,10 +56,6 @@ def test_generate_several_pullers_from_config(several_inputs_outputs_stream_conf """ Test that several inputs are correctly used to generate the related actors """ - for current_input in several_inputs_outputs_stream_config['input'].values(): - if current_input['type'] == 'csv': - current_input['files'] = current_input['files'].split(',') - generator = PullerGenerator(BroadcastReportFilter()) pullers = generator.generate(several_inputs_outputs_stream_config) diff --git a/tests/unit/cli/test_generator_influxdb2.py b/tests/unit/cli/test_generator_influxdb2.py index d2a17c0a..dffde33d 100644 --- a/tests/unit/cli/test_generator_influxdb2.py +++ b/tests/unit/cli/test_generator_influxdb2.py @@ -53,7 +53,6 @@ def influxdb2_config() -> dict: 'org': 'powerapi', 'token': 'powerapi-auth-token', 'bucket': 'powerapi-example-bucket', - 'tags': 'powerapi_example_tag1,powerapi_example_tag2' } } } diff --git a/tests/unit/cli/test_generator_prometheus.py b/tests/unit/cli/test_generator_prometheus.py index 2a7e0f56..2e7409fd 100644 --- a/tests/unit/cli/test_generator_prometheus.py +++ b/tests/unit/cli/test_generator_prometheus.py @@ -51,7 +51,7 @@ def prometheus_config() -> dict: 'model': 'PowerReport', 'addr': 'localhost', 'port': 8000, - 'tags': 'powerapi_example_tag1,powerapi_example_tag2' + 'tags': ['powerapi_example_tag1', 'powerapi_example_tag2'] } } } diff --git a/tests/utils/cli/csv_input_output_stream_mode_enabled_configuration.json b/tests/utils/cli/csv_input_output_stream_mode_enabled_configuration.json index ae07699c..0c0558bc 100644 --- a/tests/utils/cli/csv_input_output_stream_mode_enabled_configuration.json +++ b/tests/utils/cli/csv_input_output_stream_mode_enabled_configuration.json @@ -5,7 +5,7 @@ "puller": { "model": "HWPCReport", "type": "csv", - "files": "/tmp/rapl.csv,/tmp/msr.csv", + "files": ["/tmp/rapl.csv", "/tmp/msr.csv"], "name": "my_puller" } }, diff --git a/tests/utils/cli/several_inputs_outputs_stream_mode_enabled_configuration.json b/tests/utils/cli/several_inputs_outputs_stream_mode_enabled_configuration.json index 0fb609ed..6010e4b8 100644 --- a/tests/utils/cli/several_inputs_outputs_stream_mode_enabled_configuration.json +++ b/tests/utils/cli/several_inputs_outputs_stream_mode_enabled_configuration.json @@ -5,7 +5,7 @@ "puller2": { "model": "HWPCReport", "type": "csv", - "files": "/tmp/rapl.csv,/tmp/msr.csv", + "files": ["/tmp/rapl.csv", "/tmp/msr.csv"], "name": "puller2" }, "puller3": {