Skip to content

Commit

Permalink
fix: Correct bug related to arguments default values not added to con…
Browse files Browse the repository at this point in the history
…figuration when those arguments are not defined
  • Loading branch information
roda82 committed Nov 22, 2023
1 parent 8c61ef4 commit eda819c
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 10 deletions.
7 changes: 4 additions & 3 deletions powerapi/cli/common_cli_parsing_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
from powerapi.cli.parsing_manager import RootConfigParsingManager, SubgroupConfigParsingManager
from powerapi.cli.config_parser import store_true
from powerapi.cli.config_parser import MissingValueException
from powerapi.database.prometheus_db import DEFAULT_METRIC_DESCRIPTION, DEFAULT_MODEL_VALUE, DEFAULT_PUSHER_NAME

Check warning on line 35 in powerapi/cli/common_cli_parsing_manager.py

View check run for this annotation

Codecov / codecov/patch

powerapi/cli/common_cli_parsing_manager.py#L35

Added line #L35 was not covered by tests
from powerapi.exception import BadTypeException, BadContextException, UnknownArgException

POWERAPI_ENVIRONMENT_VARIABLE_PREFIX = 'POWERAPI_'
Expand Down Expand Up @@ -263,17 +264,17 @@ def __init__(self):
"d",
"metric_description",
help_text="specify metric description",
default_value="energy consumption",
default_value=DEFAULT_METRIC_DESCRIPTION
)

subparser_prometheus_output.add_argument(

Check warning on line 270 in powerapi/cli/common_cli_parsing_manager.py

View check run for this annotation

Codecov / codecov/patch

powerapi/cli/common_cli_parsing_manager.py#L270

Added line #L270 was not covered by tests
"m",
"model",
help_text="specify data type that will be stored in the database",
default_value="PowerReport",
default_value=DEFAULT_MODEL_VALUE,
)
subparser_prometheus_output.add_argument(

Check warning on line 276 in powerapi/cli/common_cli_parsing_manager.py

View check run for this annotation

Codecov / codecov/patch

powerapi/cli/common_cli_parsing_manager.py#L276

Added line #L276 was not covered by tests
"n", "name", help_text="specify pusher name", default_value="pusher_prometheus"
"n", "name", help_text="specify pusher name", default_value=DEFAULT_PUSHER_NAME
)
self.add_subgroup_parser(
subgroup_name="output",
Expand Down
2 changes: 1 addition & 1 deletion powerapi/cli/config_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ def validate(self, conf: dict) -> dict:
elif current_argument_name != 'type':
raise UnknownArgException(argument_name=current_argument_name)

return conf
return self.normalize_configuration(conf=conf)

def normalize_configuration(self, conf: dict) -> dict:
"""
Expand Down
5 changes: 4 additions & 1 deletion powerapi/database/prometheus_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@
from .base_db import BaseDB

DEFAULT_ADDRESS = '127.0.0.1'
DEFAULT_METRIC_DESCRIPTION = 'energy consumption'
DEFAULT_MODEL_VALUE = 'PowerReport'
DEFAULT_PUSHER_NAME = 'pusher_prometheus'
TAGS_KEY = 'tags'
VALUE_KEY = 'value'
TIME_KEY = 'time'
Expand All @@ -53,7 +56,7 @@ class BasePrometheusDB(BaseDB):
"""

def __init__(self, report_type: Type[Report], port: int, metric_name: str,
metric_description: str, tags: List[str], address: str = DEFAULT_ADDRESS):
tags: List[str], metric_description: str = DEFAULT_METRIC_DESCRIPTION, address: str = DEFAULT_ADDRESS):
BaseDB.__init__(self, report_type)
self.address = address
self.port = port
Expand Down
59 changes: 59 additions & 0 deletions tests/unit/cli/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
import os
import sys
from copy import deepcopy

import pytest
import tests.utils.cli as test_files_module
Expand Down Expand Up @@ -739,3 +740,61 @@ def pre_processor_binding_manager_with_reused_puller_in_bindings(
configuration=pre_processor_with_reused_puller_in_bindings_configuration)

return PreProcessorBindingManager(pullers=pullers, processors=processors)


def get_config_with_longest_argument_names(config: dict, arguments: dict):
"""
Return a copy of the provided configuration with the longest name for each argument
:param dict config: Configuration to be modified
:param dict arguments: Arguments definition
"""
config_longest_names = {}
for argument_name in config.keys():
current_argument = arguments[argument_name]
longest_argument_name = get_longest_name(current_argument.names)
config_longest_names[longest_argument_name] = config[argument_name]

return config_longest_names


def get_longest_name(names: list):
"""
Return the longest name in the provide list
:param list names: List of names
"""
longest_name = ""

for name in names:
if len(name) > len(longest_name):
longest_name = name

return longest_name


def get_config_with_default_values(config: dict, arguments: dict):
"""
Get a configuration that contains all optional arguments with their default values
:param dict config: Configuration to be modified
:param dict arguments: Arguments definition
"""

processed_arguments = []

config_default_values = deepcopy(config)

for current_argument_name, current_argument in arguments.items():
if current_argument not in processed_arguments:
argument_value_already_defined = False

for name in current_argument.names:
if name in config:
argument_value_already_defined = True

break

if not argument_value_already_defined and current_argument.default_value is not None:
config_default_values[current_argument_name] = current_argument.default_value

Check failure

Code scanning / CodeQL

Modification of parameter with default Error test

This expression mutates a
default value
.

processed_arguments.append(current_argument)

return config_default_values
14 changes: 9 additions & 5 deletions tests/unit/cli/test_config_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
SubgroupParserWithoutNameArgumentException, \
NoNameSpecifiedForSubgroupException, TooManyArgumentNamesException, MissingArgumentException, \
SameLengthArgumentNamesException, AlreadyAddedSubgroupException
from tests.unit.cli.conftest import get_config_with_longest_argument_names, get_config_with_default_values

from tests.utils.cli.base_config_parser import load_configuration_from_json_file, \
generate_configuration_tuples_from_json_file, define_environment_variables_configuration_from_json_file, \
Expand Down Expand Up @@ -145,12 +146,15 @@ def test_validate_check_mandatory_arguments_on_configuration(base_config_parser)
Test if mandatory arguments are verified by the parser
"""
conf = load_configuration_from_json_file('basic_configuration.json')
config_longest_names = get_config_with_default_values(config=conf, arguments=base_config_parser.arguments)
config_longest_names = get_config_with_longest_argument_names(config=config_longest_names,
arguments=base_config_parser.arguments)
conf_without_mandatory_arguments = \
load_configuration_from_json_file('basic_configuration_without_mandatory_arguments.json')

try:
validated_config = base_config_parser.validate(conf)
assert validated_config == conf
assert validated_config == config_longest_names

except MissingArgumentException:
assert False
Expand Down Expand Up @@ -179,12 +183,12 @@ def test_validate_adds_default_values_for_no_arguments_defined_in_configuration_
"""
conf = load_configuration_from_json_file('basic_configuration_without_arguments_with_default_values.json')

expected_conf = conf.copy()
expected_conf['arg1'] = 3
expected_conf['arg5'] = 'default value'
expected_conf_default_values = get_config_with_default_values(config=conf, arguments=base_config_parser.arguments)
expected_conf_default_values = get_config_with_longest_argument_names(config=expected_conf_default_values,
arguments=base_config_parser.arguments)

validated_config = base_config_parser.validate(conf)
assert validated_config == expected_conf
assert validated_config == expected_conf_default_values


def test_get_arguments_str_return_str_with_all_information(base_config_parser, base_config_parser_str_representation):
Expand Down

0 comments on commit eda819c

Please sign in to comment.