Skip to content
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
5 changes: 3 additions & 2 deletions src/firewheel/cli/configure_firewheel.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import os
import cmd
import shlex
import pprint
import argparse
import operator
Expand Down Expand Up @@ -43,7 +44,7 @@ def _handle_parsing(
# Print the full help message on an error
# (see: https://stackoverflow.com/a/29293080)
try:
cmd_args = parser.parse_args(args.split())
cmd_args = parser.parse_args(shlex.split(args))
except SystemExit as err:
if err.code == 2:
parser.print_help()
Expand Down Expand Up @@ -202,7 +203,7 @@ def do_set(self, args: str) -> None: # noqa: DOC502

if cmd_args.single is not None:
key = cmd_args.single[0]
value = " ".join(cmd_args.single[1:])
value = shlex.join(cmd_args.single[1:])
self.log.debug(
"Setting the FIREWHEEL config value for `%s` to `%s`.", key, value
)
Expand Down
8 changes: 6 additions & 2 deletions src/firewheel/cli/executors/shell.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,16 @@ def execute(
"MM_FORCE",
"MM_RECOVER",
"MM_CGROUP",
"MM_APPEND"
"MM_APPEND",
}

# Concatenate minimega environment variables
env_vars = [
*(f"{env}={os.environ[env]}" for env in minimega_vars if env in os.environ),
*(
f"{env}={os.environ[env]}"
for env in minimega_vars
if env in os.environ
),
f"FIREWHEEL={fw_path}",
f"FIREWHEEL_PYTHON={sys.executable}",
f"FIREWHEEL_GRPC_SERVER={grpc_path}",
Expand Down
7 changes: 4 additions & 3 deletions src/firewheel/config/_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
firewheel.log
"""

import shlex
import shutil
from typing import Any, Set, Dict, List, Final, Tuple, Union
from pathlib import Path
Expand Down Expand Up @@ -315,8 +316,8 @@ def resolve_get(
This helper method enables getting the value for a specific configuration
key. If a nested key is requested it should be represented using periods
to indicate the nesting. This function will return the Python object
of the key. Alternatively, if the value if a list, the user can return
a space separated string.
of the key. Alternatively, if the value is a list, the user can return
a space separated string.

Args:
key (str): The input key to get in *dot* notation. This means that
Expand Down Expand Up @@ -389,7 +390,7 @@ def resolve_set(
# Set the correct type of the input data
try:
if isinstance(cur_value, list):
value = value.split()
value = shlex.split(value)
elif isinstance(cur_value, bool):
value = bool(strtobool(value))
elif cur_value is not None:
Expand Down
46 changes: 45 additions & 1 deletion src/firewheel/tests/unit/cli/test_cli_configure.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def test_do_reset(self):
new_config = Config().get_config()
self.assertEqual(new_config["logging"]["cli_log"], default_setting)

def test_do_set_single(self):
def test_do_set_single_string(self):
new_log_name = "new_cli.log"
old_setting = self.old_config["logging"]["cli_log"]
self.assertNotEqual(old_setting, new_log_name)
Expand All @@ -56,6 +56,50 @@ def test_do_set_single(self):

self.assertEqual(new_config["logging"]["cli_log"], new_log_name)

def test_do_set_single_list_one_element(self):
new_nodes_string = "test_node"
old_setting = self.old_config["cluster"]["compute"]
self.assertNotEqual(old_setting, new_nodes_string)
args = f"-s cluster.compute {new_nodes_string}"
self.cli.do_set(args)

new_config = Config().get_config()

self.assertEqual(new_config["cluster"]["compute"], [new_nodes_string])

def test_do_set_single_list_one_element_with_space(self):
new_nodes_string = "test node"
old_setting = self.old_config["cluster"]["compute"]
self.assertNotEqual(old_setting, new_nodes_string)
args = f"-s cluster.compute '{new_nodes_string}'"
self.cli.do_set(args)

new_config = Config().get_config()

self.assertEqual(new_config["cluster"]["compute"], [new_nodes_string])

def test_do_set_single_list_multiple_elements(self):
new_nodes_string = "test_node0,test_node1"
old_setting = self.old_config["cluster"]["compute"]
self.assertNotEqual(old_setting, new_nodes_string)
args = f"-s cluster.compute {new_nodes_string}"
self.cli.do_set(args)

new_config = Config().get_config()

self.assertEqual(new_config["cluster"]["compute"], [new_nodes_string])

def test_do_set_single_list_multiple_elements_space(self):
new_nodes_string = "test_node0 test_node1"
old_setting = self.old_config["cluster"]["compute"]
self.assertNotEqual(old_setting, new_nodes_string)
args = f"-s cluster.compute {new_nodes_string}"
self.cli.do_set(args)

new_config = Config().get_config()

self.assertEqual(new_config["cluster"]["compute"], new_nodes_string.split(" "))

@unittest.mock.patch("sys.stderr", new_callable=io.StringIO)
@unittest.mock.patch("sys.stdout", new_callable=io.StringIO)
def test_do_set_incorrect(self, mock_stdout, mock_stderr):
Expand Down
1 change: 1 addition & 0 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ commands = coverage erase
basepython = python3
extras = format
commands =
ruff check --select I --fix {toxinidir}/src/firewheel
ruff format {toxinidir}/src/firewheel

# Linters
Expand Down
Loading