Skip to content

Commit 7c24ad0

Browse files
committed
fix for global args overwritting all of plugin args + fix for invalid plugin
1 parent 087e108 commit 7c24ad0

File tree

4 files changed

+56
-8
lines changed

4 files changed

+56
-8
lines changed

nodescraper/cli/cli.py

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,7 @@ def process_args(
327327
plugin_arg_index = -1
328328

329329
plugin_arg_map = {}
330+
invalid_plugins = []
330331
if plugin_arg_index != -1 and plugin_arg_index != len(raw_arg_input) - 1:
331332
top_level_args = raw_arg_input[: plugin_arg_index + 1]
332333
plugin_args = raw_arg_input[plugin_arg_index + 1 :]
@@ -342,7 +343,10 @@ def process_args(
342343
cur_plugin = arg
343344
elif cur_plugin:
344345
plugin_arg_map[cur_plugin].append(arg)
345-
return (top_level_args, plugin_arg_map)
346+
elif not arg.startswith("-"):
347+
# Track invalid plugin names to log event later
348+
invalid_plugins.append(arg)
349+
return (top_level_args, plugin_arg_map, invalid_plugins)
346350

347351

348352
def main(arg_input: Optional[list[str]] = None):
@@ -360,7 +364,9 @@ def main(arg_input: Optional[list[str]] = None):
360364
parser, plugin_subparser_map = build_parser(plugin_reg, config_reg)
361365

362366
try:
363-
top_level_args, plugin_arg_map = process_args(arg_input, list(plugin_subparser_map.keys()))
367+
top_level_args, plugin_arg_map, invalid_plugins = process_args(
368+
arg_input, list(plugin_subparser_map.keys())
369+
)
364370

365371
parsed_args = parser.parse_args(top_level_args)
366372
system_info = get_system_info(parsed_args)
@@ -380,6 +386,13 @@ def main(arg_input: Optional[list[str]] = None):
380386
if log_path:
381387
logger.info("Log path: %s", log_path)
382388

389+
# Log warning if invalid plugin names were provided
390+
if invalid_plugins:
391+
logger.warning(
392+
"Invalid plugin name(s) ignored: %s. Use 'describe plugin' to list available plugins.",
393+
", ".join(invalid_plugins),
394+
)
395+
383396
if parsed_args.subcmd == "summary":
384397
generate_summary(parsed_args.search_path, parsed_args.output_path, logger)
385398
sys.exit(0)

nodescraper/pluginexecutor.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
from pydantic import BaseModel
3434

3535
from nodescraper.constants import DEFAULT_LOGGER
36+
from nodescraper.enums import ExecutionStatus
3637
from nodescraper.interfaces import ConnectionManager, DataPlugin, PluginInterface
3738
from nodescraper.models import PluginConfig, SystemInfo
3839
from nodescraper.models.pluginresult import PluginResult
@@ -119,6 +120,13 @@ def run_queue(self) -> list[PluginResult]:
119120
plugin_name, plugin_args = plugin_queue.popleft()
120121
if plugin_name not in self.plugin_registry.plugins:
121122
self.logger.error("Unable to find registered plugin for name %s", plugin_name)
123+
plugin_results.append(
124+
PluginResult(
125+
status=ExecutionStatus.ERROR,
126+
source=plugin_name,
127+
message=f"Plugin '{plugin_name}' not found in registry",
128+
)
129+
)
122130
continue
123131

124132
plugin_class = self.plugin_registry.plugins[plugin_name]
@@ -140,6 +148,13 @@ def run_queue(self) -> list[PluginResult]:
140148
"Unable to find registered connection manager class for %s that is required by",
141149
connection_manager_class.__name__,
142150
)
151+
plugin_results.append(
152+
PluginResult(
153+
status=ExecutionStatus.ERROR,
154+
source=plugin_name,
155+
message=f"Connection manager '{connection_manager_class.__name__}' not found in registry",
156+
)
157+
)
143158
continue
144159

145160
if connection_manager_class not in self.connection_library:
@@ -173,13 +188,26 @@ def run_queue(self) -> list[PluginResult]:
173188
global_run_args = self.apply_global_args_to_plugin(
174189
plugin_inst, plugin_class, self.plugin_config.global_args
175190
)
191+
# Merge analysis_args and collection_args
192+
for args_key in ["analysis_args", "collection_args"]:
193+
if args_key in global_run_args and args_key in run_payload:
194+
# Merge: global args override plugin-specific args keys specified in both global and plugin-specific args
195+
run_payload[args_key].update(global_run_args[args_key])
196+
del global_run_args[args_key]
176197
run_payload.update(global_run_args)
177198
except ValueError as ve:
178199
self.logger.error(
179200
"Invalid global_args for plugin %s: %s. Skipping plugin.",
180201
plugin_name,
181202
str(ve),
182203
)
204+
plugin_results.append(
205+
PluginResult(
206+
status=ExecutionStatus.ERROR,
207+
source=plugin_name,
208+
message=f"Invalid global_args for plugin: {str(ve)}",
209+
)
210+
)
183211
continue
184212

185213
self.logger.info("-" * 50)

test/functional/test_run_plugins.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,15 @@ def test_run_all_plugins_together(run_cli_command, all_plugins, tmp_path):
8888

8989

9090
def test_run_plugin_with_invalid_name(run_cli_command):
91-
"""Test that running a non-existent plugin fails gracefully."""
91+
"""Test that running a non-existent plugin logs a warning and falls back to default config."""
9292
result = run_cli_command(["run-plugins", "NonExistentPlugin"], check=False)
9393

94-
assert result.returncode != 0
95-
output = (result.stdout + result.stderr).lower()
96-
assert "error" in output or "invalid" in output or "not found" in output
94+
# Invalid plugin is ignored and default config runs instead
95+
# Exit code depends on whether default config plugins succeed
96+
output = result.stdout + result.stderr
97+
# Check that warning was logged for invalid plugin
98+
assert "Invalid plugin name(s) ignored: NonExistentPlugin" in output
99+
# Check that default config was used
100+
assert "running default config" in output.lower() or "NodeStatus" in output
101+
# Verify it didn't crash
102+
assert "Data written to csv file" in output

test/unit/framework/test_cli.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,12 +115,12 @@ def test_system_info_builder():
115115
(
116116
["--sys-name", "test-sys", "--sys-sku", "test-sku"],
117117
["TestPlugin1", "TestPlugin2"],
118-
(["--sys-name", "test-sys", "--sys-sku", "test-sku"], {}),
118+
(["--sys-name", "test-sys", "--sys-sku", "test-sku"], {}, []),
119119
),
120120
(
121121
["--sys-name", "test-sys", "--sys-sku", "test-sku", "run-plugins", "-h"],
122122
["TestPlugin1", "TestPlugin2"],
123-
(["--sys-name", "test-sys", "--sys-sku", "test-sku", "run-plugins", "-h"], {}),
123+
(["--sys-name", "test-sys", "--sys-sku", "test-sku", "run-plugins", "-h"], {}, []),
124124
),
125125
(
126126
[
@@ -143,6 +143,7 @@ def test_system_info_builder():
143143
"TestPlugin1": ["--plugin1_arg", "test-val1"],
144144
"TestPlugin2": ["--plugin2_arg", "test-val2"],
145145
},
146+
[],
146147
),
147148
),
148149
],

0 commit comments

Comments
 (0)