Skip to content

feat: make VenvManager extensible through configurable managers - #13

Merged
raidos23 merged 9 commits into
mainfrom
feature/venv_manager
Aug 1, 2026
Merged

feat: make VenvManager extensible through configurable managers#13
raidos23 merged 9 commits into
mainfrom
feature/venv_manager

Conversation

@raidos23

@raidos23 raidos23 commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Extend VenvManager with a data-driven manager system using YAML configuration, manager detection rules, and executor-based command resolution.

raidos23 added 8 commits July 30, 2026 15:05
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>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +1962 to 1965
program, args = self._prepare_manager_command(
"add",
extra_args=["--upgrade", "pip", "setuptools", "wheel"]
+ (

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Comment on lines +1399 to +1400
if hasattr(self.parent, "_arm_process_timeout"):
self.parent._arm_process_timeout(process, timeout_ms, label)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@raidos23
raidos23 force-pushed the feature/venv_manager branch 2 times, most recently from 31f267f to a51f38b Compare August 1, 2026 22:59
…path resolution

Signed-off-by: Samuel Amen Ague <ague.samuel27@gmail.com>
@raidos23
raidos23 force-pushed the feature/venv_manager branch from bf5d9d7 to 633ce1d Compare August 1, 2026 23:18
@raidos23
raidos23 merged commit 9b3677d into main Aug 1, 2026
8 checks passed
@raidos23
raidos23 deleted the feature/venv_manager branch August 4, 2026 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant