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

break(framework) Use spaces instead of commas for separating config args #4000

Merged
merged 13 commits into from
Aug 21, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
9 changes: 4 additions & 5 deletions src/py/flwr/cli/run/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ def run(
"--run-config",
"-c",
help="Override configuration key-value pairs, should be of the format:\n\n"
"`--run-config key1=value1,key2=value2 --run-config key3=value3`\n\n"
'`--run-config \'key1="value1" key2="value2"\' '
"--run-config 'key3=\"value3\"'`\n\n"
"Note that `key1`, `key2`, and `key3` in this example need to exist "
"inside the `pyproject.toml` in order to be properly overriden.",
),
Expand Down Expand Up @@ -171,9 +172,7 @@ def _run_with_superexec(

req = StartRunRequest(
fab=fab_to_proto(fab),
override_config=user_config_to_proto(
parse_config_args(config_overrides, separator=",")
),
override_config=user_config_to_proto(parse_config_args(config_overrides)),
federation_config=user_config_to_proto(
flatten_dict(federation_config.get("options"))
),
Expand Down Expand Up @@ -214,7 +213,7 @@ def _run_without_superexec(
]

if config_overrides:
command.extend(["--run-config", f"{','.join(config_overrides)}"])
command.extend(["--run-config", f"{' '.join(config_overrides)}"])

# Run the simulation
subprocess.run(
Expand Down
4 changes: 2 additions & 2 deletions src/py/flwr/client/supernode/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -325,9 +325,9 @@ def _parse_args_common(parser: argparse.ArgumentParser) -> None:
parser.add_argument(
"--node-config",
type=str,
help="A comma separated list of key/value pairs (separated by `=`) to "
help="A space separated list of key/value pairs (separated by `=`) to "
"configure the SuperNode. "
"E.g. --node-config 'key1=\"value1\",partition-id=0,num-partitions=100'",
"E.g. --node-config 'key1=\"value1\" partition-id=0 num-partitions=100'",
)


Expand Down
18 changes: 11 additions & 7 deletions src/py/flwr/common/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"""Provide functions for managing global Flower config."""

import os
import re
from pathlib import Path
from typing import Any, Dict, List, Optional, Tuple, Union, cast, get_args

Expand Down Expand Up @@ -165,26 +166,29 @@ def unflatten_dict(flat_dict: Dict[str, Any]) -> Dict[str, Any]:

def parse_config_args(
config: Optional[List[str]],
separator: str = ",",
) -> UserConfig:
"""Parse separator separated list of key-value pairs separated by '='."""
overrides: UserConfig = {}

if config is None:
return overrides

# Regular expression to capture key-value pairs with possible quoted values
pattern = re.compile(r"(\S+?)=(\'[^\']*\'|\"[^\"]*\"|\S+)")

for config_line in config:
if config_line:
overrides_list = config_line.split(separator)
matches = pattern.findall(config_line)

if (
len(overrides_list) == 1
and "=" not in overrides_list
and overrides_list[0].endswith(".toml")
len(matches) == 1
and "=" not in matches[0][0]
and matches[0][0].endswith(".toml")
):
with Path(overrides_list[0]).open("rb") as config_file:
with Path(matches[0][0]).open("rb") as config_file:
overrides = flatten_dict(tomli.load(config_file))
else:
toml_str = "\n".join(overrides_list)
toml_str = "\n".join(f"{k} = {v}" for k, v in matches)
overrides.update(tomli.loads(toml_str))

return overrides
Expand Down
2 changes: 1 addition & 1 deletion src/py/flwr/common/config_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ def test_parse_config_args_none() -> None:
def test_parse_config_args_overrides() -> None:
"""Test parse_config_args with key-value pairs."""
assert parse_config_args(
["key1='value1',key2='value2'", "key3=1", "key4=2.0,key5=true,key6='value6'"]
["key1='value1' key2='value2'", "key3=1", "key4=2.0 key5=true key6='value6'"]
) == {
"key1": "value1",
"key2": "value2",
Expand Down
6 changes: 3 additions & 3 deletions src/py/flwr/superexec/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,9 @@ def _parse_args_run_superexec() -> argparse.ArgumentParser:
)
parser.add_argument(
"--executor-config",
help="Key-value pairs for the executor config, separated by commas. "
'For example:\n\n`--executor-config superlink="superlink:9091",'
'root-certificates="certificates/superlink-ca.crt"`',
help="Key-value pairs for the executor config, separated by spaces. "
'For example:\n\n`--executor-config \'superlink="superlink:9091" '
'root-certificates="certificates/superlink-ca.crt"\'`',
)
parser.add_argument(
"--insecure",
Expand Down