REFACTOR: Migrate CLI to use pyrit.models#1997
Conversation
rlundeen2
force-pushed
the
rlundeen2/phase-16-pyrit-models-plan
branch
from
ed441d2 to
37b5cfb
Compare
…hase 16) Add pyrit/models/catalog package (target, scenario, initializer) as the canonical typed catalog models. Rewrite the CLI api_client and output to consume typed objects instead of dict[str, Any] payloads, and update backend mappers/routes/services plus tests to import the canonical types. Remove backend re-export shims so backend models hold only REST framing types. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
rlundeen2
force-pushed
the
rlundeen2/phase-16-pyrit-models-plan
branch
from
37b5cfb to
e72af5f
Compare
|
The title and description both say |
rlundeen2
changed the title
Yep, meant to be models, sorry! |
…rit-models-plan # Conflicts: # pyrit/backend/mappers/target_mappers.py # pyrit/backend/routes/scenarios.py # pyrit/cli/_output.py # pyrit/cli/api_client.py # tests/unit/cli/test_output.py
Co-authored-by: hannahwestra25 <hannahwestra@microsoft.com>
…rit-models-plan # Conflicts: # pyrit/backend/routes/scenarios.py # pyrit/cli/_output.py # tests/unit/backend/test_scenario_run_service.py # tests/unit/cli/test_output.py
Mirrors the target.py fix: these catalog models ARE the FastAPI REST response models, so Field(..., description=...) belongs on them and surfaces in the OpenAPI schema. Removes the misleading docstring claim that descriptions live in the backend layer. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
flagging a small divergence from pyrit/backend/models/{scenarios,initializers,targets}.py keep these as re-export shims for one release. mentioned in the gist (i.e., something like from pyrit.backend.models import RegisteredScenario will break for without DeprecationWarning.)
| """Status of a scenario run, aligned with core ScenarioRunState.""" | ||
|
|
||
| CREATED = "CREATED" | ||
| INITIALIZING = "INITIALIZING" |
There was a problem hiding this comment.
noting that GHC noticed INITIALIZING was dropped. (ScenarioRunState doesn't have INTIALIZING as a status). is that not needed anymore?
| return 1 | ||
|
|
||
| if run.get("status") == "COMPLETED": | ||
| if run.status == "COMPLETED": |
There was a problem hiding this comment.
compare to Enum instead of string?
| ScenarioRunSummary, | ||
| ) | ||
|
|
||
| _TERMINAL_STATUSES = {"COMPLETED", "FAILED", "CANCELLED"} |
There was a problem hiding this comment.
should be ScenarioRunState Enums?
|
|
||
| # Print results | ||
| if run.get("status") == "COMPLETED": | ||
| if run.status == "COMPLETED": |
|
|
||
| prompt = "pyrit> " | ||
|
|
||
| _TERMINAL_STATUSES = {"COMPLETED", "FAILED", "CANCELLED"} |
| await _output.print_scenario_result_async(result_dict=detail) | ||
| await _output.print_scenario_result_async(result=detail) | ||
| except Exception: | ||
| _output.print_scenario_run_summary(run=run) |
There was a problem hiding this comment.
if the server sends back a malformed payload that can't be deserialized properly (on model_validate in get_scenario_run_results_async), the error will get swallowed here without the user knowing. (same in pyrit_shell)
There was a problem hiding this comment.
maybe log the error at least
4 participants
This PR migrates the CLI to use pyrit.models (now that it is a lot thinner) instead of dictionaries.
Also, this PR adds pyrit/models/catalog package (target, scenario, initializer) as the canonical typed catalog models - there were previously defined in the backend, but are needed by the API and are not presentation specific.
This is phase 16 of this pyrit.models plan: https://gist.github.com/rlundeen2/3e8daa8e12a11b4b6e52587b3c9b1dca