Skip to content

Commit 56d2e1e

Browse files
authored
refactor: extract parse_bool() utility for CLI boolean parameter handling (#401)
Addresses #397 - Extract boolean parsing logic from getenv_bool() into a reusable parse_bool() utility function. Changes: - Add parse_bool() to nipyapi/utils.py for handling CLI boolean parameters where fire passes --flag=false as string 'false' - Refactor getenv_bool() to use parse_bool() internally - Remove duplicate _env_bool() from ci/cleanup.py and ci/stop_flow.py - Update ci/configure_inherited_params.py to use parse_bool() - Update docs/contributing.rst CLI Boolean Parameters section with examples - Add comprehensive TestParseBool test class in test_utils.py - Remove obsolete TestEnvBool from test_ci.py (covered by new tests) The parse_bool() function handles: - String values: 'false', '0', 'no', 'off', 'n' -> False - String values: 'true', '1', 'yes', 'on', 'y' -> True - Bool passthrough: True/False unchanged - None handling with default parameter - Other types: uses Python truthiness
1 parent a5785cd commit 56d2e1e

File tree

7 files changed

+225
-95
lines changed

7 files changed

+225
-95
lines changed

docs/contributing.rst

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,8 +178,25 @@ inline with quotes. Line continuation is fine - just avoid nested bullet points.
178178
The ``fire`` CLI library passes ``--flag=false`` as the **string** ``"false"``, which is
179179
truthy in Python. This causes unexpected behavior for boolean parameters.
180180

181-
Avoid boolean parameters with default ``True`` that users need to set to ``False`` via CLI.
182-
Instead:
181+
Use ``nipyapi.utils.parse_bool()`` to safely handle boolean parameters from CLI::
182+
183+
from nipyapi.utils import parse_bool
184+
185+
def my_function(dry_run=False):
186+
# Handles CLI --dry_run=false correctly (returns False, not truthy "false")
187+
dry_run = parse_bool(dry_run, default=False)
188+
# Now safe to use as boolean
189+
190+
For environment variables, use ``nipyapi.utils.getenv_bool()`` which internally uses
191+
``parse_bool()``::
192+
193+
from nipyapi.utils import getenv_bool
194+
195+
# Returns False for "false", "0", "no", "off", "n"
196+
# Returns True for "true", "1", "yes", "on", "y", or any other non-falsy string
197+
enabled = getenv_bool("MY_FEATURE_ENABLED", default=False)
198+
199+
Alternative approaches (when ``parse_bool()`` is not suitable):
183200

184201
* Return structured results and let the caller decide (preferred)
185202
* Use ``--noflag`` syntax (fire's native boolean negation): ``nipyapi foo bar --noflag``

nipyapi/ci/cleanup.py

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# pylint: disable=broad-exception-caught,duplicate-code
1+
# pylint: disable=broad-exception-caught
22
"""
33
cleanup - stop and delete a deployed process group.
44
"""
@@ -12,16 +12,6 @@
1212
log = logging.getLogger(__name__)
1313

1414

15-
def _env_bool(name, default=False):
16-
"""Parse a boolean from environment variable."""
17-
value = os.environ.get(name, "").lower()
18-
if value in ("true", "1", "yes"):
19-
return True
20-
if value in ("false", "0", "no"):
21-
return False
22-
return default
23-
24-
2515
# pylint: disable=too-many-arguments,too-many-positional-arguments
2616
# pylint: disable=too-many-branches,too-many-statements,too-many-locals
2717
def cleanup(
@@ -81,17 +71,33 @@ def cleanup(
8171
NIFI_DELETE_PARAMETER_CONTEXT=true NIFI_FORCE_DELETE=true nipyapi ci cleanup
8272
"""
8373
# Resolve from env vars with safe defaults
74+
# Use parse_bool for CLI parameters (fire passes --flag=false as string "false")
75+
# Use getenv_bool for environment variable fallbacks
8476
process_group_id = process_group_id or os.environ.get("NIFI_PROCESS_GROUP_ID")
8577
if stop_only is None:
86-
stop_only = _env_bool("NIFI_STOP_ONLY", default=False)
78+
stop_only = nipyapi.utils.getenv_bool("NIFI_STOP_ONLY", default=False)
79+
else:
80+
stop_only = nipyapi.utils.parse_bool(stop_only, default=False)
8781
if force is None:
88-
force = _env_bool("NIFI_FORCE_DELETE", default=False)
82+
force = nipyapi.utils.getenv_bool("NIFI_FORCE_DELETE", default=False)
83+
else:
84+
force = nipyapi.utils.parse_bool(force, default=False)
8985
if delete_parameter_context is None:
90-
delete_parameter_context = _env_bool("NIFI_DELETE_PARAMETER_CONTEXT", default=False)
86+
delete_parameter_context = nipyapi.utils.getenv_bool(
87+
"NIFI_DELETE_PARAMETER_CONTEXT", default=False
88+
)
89+
else:
90+
delete_parameter_context = nipyapi.utils.parse_bool(delete_parameter_context, default=False)
9191
if delete_orphaned_contexts is None:
92-
delete_orphaned_contexts = _env_bool("NIFI_DELETE_ORPHANED_CONTEXTS", default=False)
92+
delete_orphaned_contexts = nipyapi.utils.getenv_bool(
93+
"NIFI_DELETE_ORPHANED_CONTEXTS", default=False
94+
)
95+
else:
96+
delete_orphaned_contexts = nipyapi.utils.parse_bool(delete_orphaned_contexts, default=False)
9397
if disable_controllers is None:
94-
disable_controllers = _env_bool("NIFI_DISABLE_CONTROLLERS", default=True)
98+
disable_controllers = nipyapi.utils.getenv_bool("NIFI_DISABLE_CONTROLLERS", default=True)
99+
else:
100+
disable_controllers = nipyapi.utils.parse_bool(disable_controllers, default=True)
95101

96102
if not process_group_id:
97103
raise ValueError("process_group_id is required (or set NIFI_PROCESS_GROUP_ID)")

nipyapi/ci/configure_inherited_params.py

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -65,16 +65,17 @@ def configure_inherited_params(
6565
parameters = parameters or os.environ.get("NIFI_PARAMETERS")
6666
parameters_file = parameters_file or os.environ.get("NIFI_PARAMETERS_FILE")
6767

68-
# Parse boolean env vars
69-
if isinstance(dry_run, str):
70-
dry_run = dry_run.lower() in ("true", "1", "yes")
71-
if os.environ.get("NIFI_DRY_RUN", "").lower() in ("true", "1", "yes"):
72-
dry_run = True
73-
74-
if isinstance(allow_override, str):
75-
allow_override = allow_override.lower() in ("true", "1", "yes")
76-
if os.environ.get("NIFI_ALLOW_OVERRIDE", "").lower() in ("true", "1", "yes"):
77-
allow_override = True
68+
# Parse boolean parameters using parse_bool (handles CLI --flag=false as string "false")
69+
# Environment variables can override CLI defaults
70+
dry_run = nipyapi.utils.parse_bool(dry_run, default=False)
71+
env_dry_run = nipyapi.utils.getenv_bool("NIFI_DRY_RUN", default=None)
72+
if env_dry_run is not None:
73+
dry_run = env_dry_run
74+
75+
allow_override = nipyapi.utils.parse_bool(allow_override, default=False)
76+
env_allow_override = nipyapi.utils.getenv_bool("NIFI_ALLOW_OVERRIDE", default=None)
77+
if env_allow_override is not None:
78+
allow_override = env_allow_override
7879

7980
if not process_group_id:
8081
raise ValueError("process_group_id is required (or set NIFI_PROCESS_GROUP_ID)")

nipyapi/ci/stop_flow.py

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
# pylint: disable=duplicate-code
21
"""
32
stop_flow - stop a running process group.
43
"""
@@ -12,16 +11,6 @@
1211
log = logging.getLogger(__name__)
1312

1413

15-
def _env_bool(name, default=False):
16-
"""Parse a boolean from environment variable."""
17-
value = os.environ.get(name, "").lower()
18-
if value in ("true", "1", "yes"):
19-
return True
20-
if value in ("false", "0", "no"):
21-
return False
22-
return default
23-
24-
2514
def stop_flow(
2615
process_group_id: Optional[str] = None,
2716
disable_controllers: Optional[bool] = None,
@@ -56,9 +45,13 @@ def stop_flow(
5645
# Stop for deletion (also disable controllers)
5746
nipyapi ci stop_flow --process_group_id PG_ID --disable_controllers
5847
"""
48+
# Use parse_bool for CLI parameters (fire passes --flag=false as string "false")
49+
# Use getenv_bool for environment variable fallbacks
5950
process_group_id = process_group_id or os.environ.get("NIFI_PROCESS_GROUP_ID")
6051
if disable_controllers is None:
61-
disable_controllers = _env_bool("NIFI_DISABLE_CONTROLLERS", default=False)
52+
disable_controllers = nipyapi.utils.getenv_bool("NIFI_DISABLE_CONTROLLERS", default=False)
53+
else:
54+
disable_controllers = nipyapi.utils.parse_bool(disable_controllers, default=False)
6255

6356
if not process_group_id:
6457
raise ValueError("process_group_id is required (or set NIFI_PROCESS_GROUP_ID)")

nipyapi/utils.py

Lines changed: 63 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
"extract_oidc_user_identity",
4747
"getenv",
4848
"getenv_bool",
49+
"parse_bool",
4950
"resolve_relative_paths",
5051
"format_timestamp",
5152
]
@@ -921,12 +922,70 @@ def getenv(name: str, default: Optional[str] = None) -> Optional[str]:
921922
return val if val is not None else default
922923

923924

925+
def parse_bool(value, default=None): # pylint: disable=too-many-return-statements
926+
"""
927+
Parse a value as boolean.
928+
929+
Handles string values from CLI parameters (where fire passes --flag=false
930+
as the string "false") and environment variables. Also passes through
931+
actual bool values unchanged.
932+
933+
This function addresses the CLI boolean parameter gotcha where fire passes
934+
--flag=false as the string "false", which is truthy in Python. Use this
935+
to safely convert CLI parameters to boolean.
936+
937+
Args:
938+
value: The value to parse (str, bool, or None)
939+
default: Value to return if input is None
940+
941+
Returns:
942+
bool or default value if input is None
943+
944+
Example::
945+
946+
>>> parse_bool("false") # False (handles CLI --flag=false)
947+
>>> parse_bool("0") # False
948+
>>> parse_bool("yes") # True
949+
>>> parse_bool(True) # True (passthrough)
950+
>>> parse_bool(None, default=False) # False
951+
"""
952+
# Handle None
953+
if value is None:
954+
return default
955+
956+
# Pass through actual booleans unchanged
957+
if isinstance(value, bool):
958+
return value
959+
960+
# Handle string values
961+
if isinstance(value, str):
962+
val_clean = value.strip().lower()
963+
964+
# Handle empty string as falsy
965+
if val_clean == "":
966+
return False
967+
968+
# Handle JSON-style booleans directly
969+
if val_clean in ("true", "false"):
970+
return json.loads(val_clean)
971+
972+
# Handle common falsy patterns
973+
if val_clean in ("0", "no", "off", "n"):
974+
return False
975+
976+
# Everything else is truthy (including '1', 'yes', 'on', 'y', etc.)
977+
return True
978+
979+
# For other types, use Python's truthiness
980+
return bool(value)
981+
982+
924983
def getenv_bool(name: str, default: Optional[bool] = None) -> Optional[bool]:
925984
"""
926-
Parse environment variable as boolean using JSON-style interpretation.
985+
Parse environment variable as boolean.
927986
928-
Handles common boolean environment variable patterns and uses json.loads()
929-
for the standard 'true'/'false' cases that most programmers understand.
987+
Convenience wrapper around parse_bool() for environment variables.
988+
Handles common boolean environment variable patterns.
930989
931990
Args:
932991
name (str): Environment variable name
@@ -946,20 +1005,7 @@ def getenv_bool(name: str, default: Optional[bool] = None) -> Optional[bool]:
9461005
val = os.getenv(name)
9471006
if val is None:
9481007
return default
949-
950-
# Clean and normalize the value
951-
val_clean = val.strip().lower()
952-
953-
# Handle JSON-style booleans directly
954-
if val_clean in ("true", "false"):
955-
return json.loads(val_clean)
956-
957-
# Handle common falsy patterns
958-
if val_clean in ("0", "no", "off", "n", ""):
959-
return False
960-
961-
# Everything else is truthy (including '1', 'yes', 'on', 'y', etc.)
962-
return True
1008+
return parse_bool(val)
9631009

9641010

9651011
def format_timestamp(ts=None, fmt=None):

tests/test_ci.py

Lines changed: 2 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -13,44 +13,8 @@
1313
# Pure Logic Tests (no NiFi required)
1414
# =============================================================================
1515

16-
17-
class TestEnvBool:
18-
"""Test _env_bool helper function used by stop_flow and cleanup."""
19-
20-
def test_env_bool_true_values(self):
21-
"""Test that true-like values return True."""
22-
from nipyapi.ci.stop_flow import _env_bool
23-
24-
with patch.dict(os.environ, {"TEST_VAR": "true"}):
25-
assert _env_bool("TEST_VAR") is True
26-
with patch.dict(os.environ, {"TEST_VAR": "1"}):
27-
assert _env_bool("TEST_VAR") is True
28-
with patch.dict(os.environ, {"TEST_VAR": "yes"}):
29-
assert _env_bool("TEST_VAR") is True
30-
with patch.dict(os.environ, {"TEST_VAR": "TRUE"}):
31-
assert _env_bool("TEST_VAR") is True
32-
33-
def test_env_bool_false_values(self):
34-
"""Test that false-like values return False."""
35-
from nipyapi.ci.stop_flow import _env_bool
36-
37-
with patch.dict(os.environ, {"TEST_VAR": "false"}):
38-
assert _env_bool("TEST_VAR") is False
39-
with patch.dict(os.environ, {"TEST_VAR": "0"}):
40-
assert _env_bool("TEST_VAR") is False
41-
with patch.dict(os.environ, {"TEST_VAR": "no"}):
42-
assert _env_bool("TEST_VAR") is False
43-
44-
def test_env_bool_default(self):
45-
"""Test default value when env var not set."""
46-
from nipyapi.ci.stop_flow import _env_bool
47-
48-
# Clear the var if it exists
49-
env = os.environ.copy()
50-
env.pop("NONEXISTENT_VAR", None)
51-
with patch.dict(os.environ, env, clear=True):
52-
assert _env_bool("NONEXISTENT_VAR", default=False) is False
53-
assert _env_bool("NONEXISTENT_VAR", default=True) is True
16+
# Note: Tests for parse_bool and getenv_bool are in test_utils.py::TestParseBool
17+
# and test_utils.py::TestGetenvBool respectively.
5418

5519

5620
class TestMaskValue:

0 commit comments

Comments
 (0)