feat: make VenvManager extensible through configurable managers - #13
Conversation
Signed-off-by: Samuel Amen Ague <ague.samuel27@gmail.com>
- Connect VenvManager._prepare_manager_command to ExecutorFactory - Support both list and dict formats for manager commands in VenvManagerConfig - Add create_venv command definition to VenvManagers.yml - Add unit tests for ExecutorFactory and VenvManager command preparation Signed-off-by: Samuel Amen Ague <ague.samuel27@gmail.com>
Signed-off-by: Samuel Amen Ague <ague.samuel27@gmail.com>
- Implement action-specific executor retrieval in VenvManagerConfig. - Add methods for detecting the default manager and resolving the manager for a workspace. - Update VenvManagers.yml to define commands for both pip and poetry. - Modify VenvManager to utilize dynamic detection and user preferences for manager selection. - Introduce unit tests for workspace manager detection and preference handling. Signed-off-by: Samuel Amen Ague <ague.samuel27@gmail.com>
Signed-off-by: Samuel Amen Ague <ague.samuel27@gmail.com>
- Remove PySide6/Qt imports and QProcess/QTimer dependencies from Core/Venv_Manager/Manager.py - Remove _ui_callbacks dictionary and _call_ui delegate invocation pattern - Implement protected virtual hooks (_tr, _show_progress, _close_progress, etc.) in VenvManager - Override virtual hooks in VenvManagerUI for PySide6 GUI progress and message handling - Ensure 100% pure core decoupled architecture with 62/62 passing tests Signed-off-by: Samuel Amen Ague <ague.samuel27@gmail.com>
…`internet` to `os_helpers` Signed-off-by: Samuel Amen Ague <ague.samuel27@gmail.com>
…tion - Introduce `resolve_venv_path` method in VenvManagerConfig to dynamically resolve virtual environment paths based on YAML configuration. - Implement `WorkspacePathExecutor` to handle workspace-relative path resolution. - Update VenvManager to utilize the new path resolution method and improve command preparation logic. - Modify VenvManagers.yml to include `get_venv_path` command for workspace path retrieval. - Enhance unit tests to cover new functionality and ensure robust behavior. Signed-off-by: Samuel Amen Ague <ague.samuel27@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ad25ace33
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| program, args = self._prepare_manager_command( | ||
| "add", | ||
| extra_args=["--upgrade", "pip", "setuptools", "wheel"] | ||
| + ( |
There was a problem hiding this comment.
Route pip bootstrapping through pip
When a workspace is detected as Poetry, this call builds poetry add --upgrade pip setuptools wheel; in the inspected Poetry CLI help, add has no --upgrade option, so the dependency-install flow fails during the bootstrap phase before project dependencies are installed for any Poetry project. The ensurepip/upgrade phase should continue to use the selected environment's Python/pip instead of the active manager's add command unless that manager explicitly supports the same semantics.
Useful? React with 👍 / 👎.
| if hasattr(self.parent, "_arm_process_timeout"): | ||
| self.parent._arm_process_timeout(process, timeout_ms, label) |
There was a problem hiding this comment.
Restore the core timeout fallback
This now only delegates to self.parent._arm_process_timeout, but a repo-wide search shows no GUI or CLI parent implements that hook, so all existing calls that arm timeouts for venv creation, pip show, and dependency installs become no-ops. If one of those subprocesses hangs, it will no longer be killed as the previous core QTimer implementation did; keep a fallback timer here when the parent does not provide an override.
Useful? React with 👍 / 👎.
31f267f to
a51f38b
Compare
…path resolution Signed-off-by: Samuel Amen Ague <ague.samuel27@gmail.com>
bf5d9d7 to
633ce1d
Compare
Extend VenvManager with a data-driven manager system using YAML configuration, manager detection rules, and executor-based command resolution.