fix: fail locally when no identifier is given#214
Conversation
mmilanta
force-pushed
the
fix/fail-on-cli-when-no-identifier-is-given
branch
from
4bb25c1 to
0293ae5
Compare
src/agent_scan/cli.py
Outdated
|
|
||
| i += 1 | ||
| if identifier is None: | ||
| raise ValueError(f"Control server {url} is missing a --control-identifier") |
There was a problem hiding this comment.
This will also print a full traceback.
With something like this: https://github.com/snyk/agent-scan/blob/main/src/agent_scan/verify_api.py#L212
and https://github.com/snyk/agent-scan/blob/main/src/agent_scan/run.py#L11-L12
We can avoid the full stack trace. Maybe some sort of a common ValidationError that can be reused for other CLI flag validations? This will look cleaner.
| def parse_control_servers(argv): | ||
| def parse_control_servers(argv) -> list[ControlServer]: | ||
| """ | ||
| Parse control server arguments from sys.argv. |
There was a problem hiding this comment.
It would be nice to explain the expectation here in a comment.
We are looking for blocks of --control-server, --control-server-H, --control-identifier flags. We expect these flags to be together in the same block one after the other before the next such block begins.
Ignore if this block behavior is already part of --help otherwise maybe another place to clarify this expectation is in --help.
| identifier: str | None = None | ||
|
|
||
| i = 2 | ||
| while i < len(block): |
There was a problem hiding this comment.
Is there a possibility of an infinite loop here?
In the while loop, we only increment i if we successfully match --control-server-H or --control-identifier and the following argument does not start with --.
If our block contains an unrecognized flag like --verbose, or if a flag is missing its value like
--control-identifier --some-other-flag, both the if elif conditions will fail - and the loop will be stuck forever?
We were earlier doing i += 1 in an else.
It would be nice to have a test for such cases.
There was a problem hiding this comment.
but there is else, i+=1
2 participants
No description provided.