Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the codebase by renaming "scenarios" to "runners" and "scenarioruns" to "runs" throughout the API structure. It also removes the deprecated ABBA command module and updates authentication from Azure tokens to Keycloak tokens.
- Renames scenario-related API endpoints and services to runner-related equivalents
- Replaces scenariorun functionality with simplified run functionality
- Removes the entire ABBA module for batch scenario processing
- Switches authentication mechanism from Azure to Keycloak tokens
Reviewed Changes
Copilot reviewed 35 out of 40 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| Babylon/test/api/runs/test_run_service.py | Updates test class and imports for new run service, removes cumulated_logs test |
| Babylon/test/api/runners/test_runner_service.py | Updates test class and imports for new runner service, changes test data references |
| Babylon/commands/macro/destroy.py | Updates import and service instantiation for runners |
| Babylon/commands/api/scenarios/apply.py | Removes entire scenario apply command file |
| Babylon/commands/api/scenarioruns/ | Removes all scenariorun-related files and services |
| Babylon/commands/api/runs/ | Adds new run-related commands (logs, status, stop) with Keycloak auth |
| Babylon/commands/api/runners/ | Adds new runner-related commands and services with Keycloak auth |
| Babylon/commands/abba/ | Removes entire ABBA module for batch processing |
| Babylon/commands/init.py | Removes ABBA from command groups |
| .github/workflows/ci.yml | Updates CI tests to include runs and runners |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
The log message still references 'Scenario run' but should be updated to 'Runner run' to match the refactoring from scenarios to runners.
| logger.info(f"Scenario run {runner_run['id']} has been successfully added to state") | |
| logger.info(f"Runner run {runner_run['id']} has been successfully added to state") |
There was a problem hiding this comment.
The log messages still reference 'Scenario' but should be updated to 'Runner' to match the refactoring from scenarios to runners.
| logger.info(f'Scenario {service_state["api"]["runner_id"]} successfully updated') | |
| if service_state["api"]["runner_id"] == state["services"]["api"]["runner_id"]: | |
| logger.info(f'Scenario {state["services"]["api"]["runner_id"]} stored in state has been successfully updated') | |
| logger.info(f'Runner {service_state["api"]["runner_id"]} successfully updated') | |
| if service_state["api"]["runner_id"] == state["services"]["api"]["runner_id"]: | |
| logger.info(f'Runner {state["services"]["api"]["runner_id"]} stored in state has been successfully updated') |
There was a problem hiding this comment.
The log messages still reference 'Scenario' but should be updated to 'Runner' to match the refactoring from scenarios to runners.
There was a problem hiding this comment.
The log messages still reference 'Scenario' but should be updated to 'Runner' to match the refactoring from scenarios to runners.
There was a problem hiding this comment.
The log message still references 'Scenario' but should be updated to 'Runner' to match the refactoring from scenarios to runners.
| logger.info(f"Scenario {runner['id']} has been successfully added in state") | |
| logger.info(f"Runner {runner['id']} has been successfully added in state") |
There was a problem hiding this comment.
The error message refers to 'solution id' but should refer to 'run id' based on the context and the check for self.run_id on line 20.
| logger.error("solution id is missing") | |
| logger.error("run id is missing") |
a1fe047 to
4041988
Compare
No description provided.