feat: Add Python Virtual Environment Support: Installing User Defined Packages#4902
Conversation
kunwp1
left a comment
There was a problem hiding this comment.
The feature is working well! I left a few comments to improve the codebase and user experience.
…3/texera into pve-add-user-packages
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4902 +/- ##
============================================
- Coverage 42.78% 42.72% -0.07%
+ Complexity 2189 2186 -3
============================================
Files 1045 1045
Lines 39876 39979 +103
Branches 4205 4218 +13
============================================
+ Hits 17062 17080 +18
- Misses 21757 21838 +81
- Partials 1057 1061 +4
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
kunwp1
left a comment
There was a problem hiding this comment.
Left a few more comments.
…3/texera into pve-add-user-packages
…3/texera into pve-add-user-packages
kunwp1
left a comment
There was a problem hiding this comment.
Left more comments. Can you add more test cases that Codecov Report complains?
Also, this PR lacks sync check between requirements.txt and system-requirements-lock.txt. It's better to add a sync check in the github CI but totally up to you. If you are not going to add a github CI check on this PR, please mention it in the PR description and create an issue.
|
Thanks for the comments. I reviewed the Codecov report, and the current I also created issue #5034 to track adding a CI sync check between |
What changes were proposed in this PR?
This PR is an extension of PR #4484. Previously, we introduced support for creating Python Virtual Environments (PVEs) with system-level dependencies preinstalled. This PR builds on that foundation by enabling users to install custom Python packages within a PVE. In some cases, installing a user package may overwrite system-level packages. For this reason, System package versions are locked to prevent these conflicts.
Some duplicate lines of code (previously introduced in PR #4484) were removed from PveManager to improve clarity and reduce redundancy.
Any related issues, documentation, discussions?
This change is part of ongoing efforts to support environment isolation and reproducibility within Texera. Related issue includes #4296. This PR closes sub-issue #4465.
A GitHub CI sync check between
requirements.txtandsystem-requirements-lock.txtwill be added in a follow-up PR. Tracking issue: #5034.How was this PR tested?
Tested Manually and PveResourceSpec test file updated.
To test:
To test system dependency conflit management, install the following packages:
Was this PR authored or co-authored using generative AI tooling?
Co-authored using: ChatGPT (OpenAI)