NetworkPlugin update: adding url collector arg and test enhancements #120
Conversation
jaspals3123
requested review from
alexandraBara and
landrews-amd
as code owners
| Returns: | ||
| bool: True if network is accessible, False otherwise | ||
| """ | ||
| cmd = "ping" |
There was a problem hiding this comment.
this needs to be defined as a class var like "CMD_PING" so it can get picked up by the auto doc generation script.
| # Run ping command | ||
| result = self._run_sut_cmd(f"{cmd} {url} {ping_option}") | ||
|
|
||
| network_accessible = result.exit_code == 0 |
There was a problem hiding this comment.
we dont need this extra var here, just use the result like:
if result.exit_code == 0:....
|
Please update the functional test for this plugin and add the collector arg to the plugin_config: https://github.com/amd/node-scraper/blob/development/test/functional/fixtures/network_plugin_config.json |
alexandraBara
changed the title
|
|
||
|
|
||
| class NetworkCollectorArgs(CollectorArgs): | ||
| url: Optional[str] = None |
There was a problem hiding this comment.
we need another cmd here, it needs to be an optional with these choices:
if cmd not in {"ping", "wget", "curl"}:
raise ValueError("Invalid command")
| uncollected_commands, | ||
| ) | ||
|
|
||
| def _check_network_connectivity(self, url: str) -> bool: |
There was a problem hiding this comment.
this check needs to be optional based on whether the user has specified collector_arg.url
We need a logic like this:
if args.url:
network_cmd = args.network_cmd
if not network_cmd:
network_cmd = "ping"
--> now run _check_network_connectivity(url, network_cmd)
the logic is this: if user specifies url, then we need to check connectivity. If url is specified and network_cmd is not specified, then default network_cmd to "ping".
| network_accessible: Optional[bool] = None | ||
|
|
||
| # Check network connectivity if URL is provided | ||
| if args and args.url: |
There was a problem hiding this comment.
my comment above should be added here to this if logic: https://github.com/amd/node-scraper/pull/120/changes#r2760251742
No description provided.