feat(cli): add --process-type option to override process execution mode#183
feat(cli): add --process-type option to override process execution mode#183
Conversation
Co-authored-by: toby-coleman <13170610+toby-coleman@users.noreply.github.com>
Co-authored-by: toby-coleman <13170610+toby-coleman@users.noreply.github.com>
|
Benchmark comparison for |
toby-coleman
left a comment
There was a problem hiding this comment.
@copilot review the implementation in this PR with a view to simplification.
Can you reduce the amount of hard-coded if/else branches checking for the different process types, and instead do something more elegant? Can some of the logic be done in the relevant schemas?
- Add RAY_CONNECTOR_CLS_PATH constant for RayConnector - Implement ProcessSpec.override_process_type() method with mapping-based logic - Reduce CLI code from ~70 lines to ~10 lines using the new method - Fix test to check stderr instead of stdout for error messages Agent-Logs-Url: https://github.com/plugboard-dev/plugboard/sessions/2bf67b70-4b61-456d-931e-490aa60fe64a Co-authored-by: toby-coleman <13170610+toby-coleman@users.noreply.github.com>
I've refactored the implementation to simplify the logic as requested: Changes in commit 88d9c91:
The logic is now in the schema layer where it belongs, and the implementation is much more maintainable and extensible. |
- Change process_type parameter to use Literal["local", "ray"] type - Remove manual validation code (typer handles it automatically) - Remove test for invalid process type (typer validates at CLI level) Agent-Logs-Url: https://github.com/plugboard-dev/plugboard/sessions/48ff2171-dd27-42e0-9355-aa52813afa7a Co-authored-by: toby-coleman <13170610+toby-coleman@users.noreply.github.com>
|
Benchmark comparison for |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Benchmark comparison for |
Summary
Adds
--process-typeCLI option toplugboard process runfor switching betweenLocalProcessandRayProcessexecution modes, overriding YAML configuration. UsesLiteraltype for type-safe validation.Changes
CLI Enhancement
--process-type {local|ray}option toruncommand withLiteral["local", "ray"]typeSchema Enhancement
RAY_CONNECTOR_CLS_PATHconstant to connector schema for consistencyProcessSpec.override_process_type()method with mapping-based logicray:LocalProcess→RayProcess,AsyncioConnector→RayConnector,DictStateBackend→RayStateBackendlocal: reverses the above transformationsRayConnectorwhen already on Ray)Tests
Example usage:
Implementation Note: The override logic is implemented in the schema layer (
ProcessSpec.override_process_type()) using a mapping-based approach rather than hard-coded if/else branches in the CLI, keeping business logic close to the data model and improving maintainability. Type safety is enforced usingLiteraltypes, allowing typer to handle validation automatically.Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.