feat: Make Python Virtual Environment Persistent: Add Environments to Left Panel #5577
Open
SarahAsad23 wants to merge 23 commits into
Open
feat: Make Python Virtual Environment Persistent: Add Environments to Left Panel #5577SarahAsad23 wants to merge 23 commits into
SarahAsad23 wants to merge 23 commits into
Conversation
github-actions
Bot
added
engine
ddl-change
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #5577 +/- ##
============================================
- Coverage 52.39% 52.38% -0.01%
- Complexity 2478 2480 +2
============================================
Files 1070 1070
Lines 41360 41428 +68
Branches 4441 4455 +14
============================================
+ Hits 21671 21703 +32
- Misses 18416 18456 +40
+ Partials 1273 1269 -4
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
kunwp1
requested changes
Jun 10, 2026
kunwp1
left a comment
kunwp1
left a comment
Contributor
There was a problem hiding this comment.
The feature looks good to me! I left a few comments. One main decision is the naming of this module. Do we still want to use Pve or use a different name like "Environment" or "Virtual Environment"? Need a discussion.
…into add-pve-panel
…into add-pve-panel
kunwp1
reviewed
Jun 10, 2026
kunwp1
left a comment
kunwp1
left a comment
Contributor
There was a problem hiding this comment.
- Update the PR description to be up-to-date. I still see old table names.
- Add more test cases. Try to achieve the coverage report to be near 100%. In our Apache monthly meeting, we have a goal to achieve near 90% coverage and your PR is currently lowering the coverage.
- Update
changelog.xmlbecause you're changing the DDL. - Please explicitly mention in the PR that you're only persisting the metadata of the virtual environments so that the users don't confuse their CRUD on the venv won't drive anything.
Comment on lines
+139
to
+147
| try { | ||
| val packagesJson = mapper.writeValueAsString(payload.packages) | ||
| val veid = PveManager.savePve(sessionUser.getUid.intValue(), name, packagesJson) | ||
| Response.ok(Map("veid" -> veid).asJava).build() | ||
| } catch { | ||
| case e: Exception => | ||
| logger.error("Failed to save PVE", e) | ||
| throw new InternalServerErrorException(s"Failed to save PVE: ${e.getMessage}") | ||
| } |
Contributor
There was a problem hiding this comment.
Try to return 409 error for unique constraint violations instead of 500 error. Same for updatePve. Also, better to return 201 instead of 200 for success.
Comment on lines
+60
to
+61
| def isValidPveName(name: String): Boolean = | ||
| name != null && SafePveName.pattern.matcher(name).matches() |
Contributor
There was a problem hiding this comment.
The database column is VARCHAR(128) but isValidPveName only checks the charset. Add a length guard here (e.g. name.length <= 128).
| <div | ||
| *ngIf="pves.length === 0" | ||
| class="python-env-page-empty"> | ||
| No environments yet. Click <strong>Create Python Venv</strong> to create one. |
Contributor
There was a problem hiding this comment.
This says “Create Python Venv”, but the actual button (and the page title) is “Create Environment” / “Environments”. Suggest aligning the names.
| nzWrapClassName="pve-modal" | ||
| nzClassName="pve-modal" | ||
| [nzVisible]="pveModalVisible" | ||
| nzTitle="Python Environments" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this PR?
This PR introduces persistent Python Virtual Environments (PVEs) by moving them out of the Computing Unit (CU) lifecycle and storing them in the database.
Previously, PVEs were managed through Computing Units and existed only within the CU they were created in. As a result, PVEs were lost when the corresponding CU was terminated. This PR adds a new python_virtual_environments table to persist PVE configurations and introduces a dedicated dashboard interface for managing them.
Users can now create, view, update, and delete their own Python virtual environments through a new "Environments" page in the dashboard sidebar. PVE definitions are stored as user-owned resources in the database and can be managed independently of Computing Units.
K8s configurations for this feature will be added in a future PR.
Any related issues, documentation, discussions?
Related discussions and issues: #5360, #5361.
How was this PR tested?
Tested manually and tests added to PveResourceSpec.
Was this PR authored or co-authored using generative AI tooling?
Co-authored using: Claude Code