kunwp1 commented on code in PR #4902:
URL: https://github.com/apache/texera/pull/4902#discussion_r3225363045
##########
amber/src/main/scala/org/apache/texera/web/resource/pythonvirtualenvironment/PveManager.scala:
##########
@@ -61,9 +67,51 @@ object PveManager {
"PIP_NO_INPUT" -> "1"
)
- def getSystemPackages(): Seq[String] = {
- val python = PythonUtils.getPythonExecutable
- Process(Seq(python, "-m", "pip",
"freeze")).!!.split("\n").map(_.trim).filter(_.nonEmpty).toSeq
+ private def getSystemPath(isLocal: Boolean): Path = {
+ if (isLocal) {
+ Paths.get("amber", "system-requirements-lock.txt")
+ } else {
+ Paths.get("/tmp", "system-requirements-lock.txt")
+ }
+ }
Review Comment:
Merge two `Paths.get()` to a single one.
##########
amber/system-requirements-lock.txt:
##########
@@ -0,0 +1,113 @@
+aiobotocore==2.25.1
+aiohappyeyeballs==2.6.1
+aiohttp==3.13.5
+aioitertools==0.13.0
+aiosignal==1.4.0
+annotated-types==0.7.0
+appdirs==1.4.4
+asn1crypto==1.5.1
+attrs==26.1.0
+betterproto==2.0.0b7
+bidict==0.22.0
+boto3==1.40.53
+botocore==1.40.53
+cached-property==1.5.2
+cachetools==6.2.6
+certifi==2026.4.22
+charset-normalizer==3.4.7
+click==8.3.3
+contourpy==1.3.3
+cycler==0.12.1
+Deprecated==1.2.14
+filelock==3.29.0
+fonttools==4.62.1
+frozenlist==1.8.0
+fs==2.4.16
+fsspec==2025.9.0
+grpclib==0.4.9
+h2==4.3.0
+hf-xet==1.5.0
+hpack==4.1.0
+huggingface_hub==0.36.2
+hyperframe==6.1.0
+idna==3.14
+ImageIO==2.37.3
+iniconfig==1.1.1
+Jinja2==3.1.6
+jmespath==1.1.0
+joblib==1.5.3
+kiwisolver==1.5.0
+lazy-loader==0.5
+loguru==0.7.0
+markdown-it-py==4.2.0
+MarkupSafe==3.0.3
+matplotlib==3.10.9
+mdurl==0.1.2
+mmh3==5.2.1
+mpmath==1.3.0
+multidict==6.7.1
+networkx==3.6.1
+numpy==2.1.0
+overrides==7.4.0
+packaging==26.2
+pampy==0.3.0
+pandas==2.2.3
+pg8000==1.31.5
+pillow==12.2.0
+plotly==5.24.1
+pluggy==1.6.0
+praw==7.6.1
+prawcore==2.4.0
+propcache==0.5.2
+protobuf==7.34.1
+psutil==5.9.0
+pyarrow==21.0.0
+pybase64==1.3.2
+pydantic==2.13.4
+pydantic_core==2.46.4
+Pygments==2.20.0
+pyiceberg==0.11.1
+Pympler==1.1
+pyparsing==3.3.2
+pyroaring==1.1.0
+pytest==7.4.0
+pytest-reraise==2.1.2
+pytest-timeout==2.2.0
+python-dateutil==2.8.2
+pytz==2026.2
+PyYAML==6.0.3
+readerwriterlock==1.0.9
+regex==2026.5.9
+requests==2.34.0
+rich==14.3.4
+ruff==0.14.7
+s3fs==2025.9.0
+s3transfer==0.14.0
+safetensors==0.7.0
+scikit-image==0.25.2
+scikit-learn==1.5.0
+scipy==1.17.1
+scramp==1.4.8
+setuptools==80.10.2
+six==1.17.0
+SQLAlchemy==2.0.37
+strictyaml==1.7.3
+sympy==1.14.0
+tenacity==8.5.0
+threadpoolctl==3.6.0
+tifffile==2026.5.2
+tokenizers==0.22.2
+torch==2.8.0
+tqdm==4.67.3
+transformers==4.57.3
+typing-inspection==0.4.2
+typing_extensions==4.14.1
+tzdata==2026.2
+tzlocal==2.1
+update-checker==0.18.0
+urllib3==2.7.0
+websocket-client==1.9.0
+wordcloud==1.9.3
Review Comment:
`wordcloud` shouldn't be in this lock.txt file. Looks like you also included
`operator-requirements.txt`. System requirements should only include
dependencies using `requirements.txt` only. Can you recreate this file?
##########
amber/src/main/scala/org/apache/texera/web/resource/pythonvirtualenvironment/PveResource.scala:
##########
@@ -28,24 +28,31 @@ import java.util
@Consumes(Array(MediaType.APPLICATION_JSON))
class PveResource {
// --------------------------------------------------
- // Get installed packages
+ // Get system packages
// --------------------------------------------------
@GET
@Path("/system")
@Produces(Array(MediaType.APPLICATION_JSON))
def getSystemPackages: util.Map[String, util.List[String]] = {
try {
- val systemPkgs = PveManager.getSystemPackages().toList.asJava
+
+ val isLocal = true
Review Comment:
Add a TODO comment that you will work on kubernetes environment.
##########
amber/src/main/scala/org/apache/texera/web/resource/pythonvirtualenvironment/PveManager.scala:
##########
@@ -212,4 +256,103 @@ object PveManager {
stream.close()
}
}
+
+ /**
+ * Installs user requested Python packages into the PVE.
+ *
+ * 1. Executes pip install for each package
+ * 2. Prevents conflicts with system dependencies.
+ * 3. Updates user metadata file
+ * 4. Streams logs back via queue
+ */
+ def installUserPackages(
+ packages: List[String],
+ cuid: Int,
+ queue: BlockingQueue[String],
+ pveName: String,
+ isLocal: Boolean
+ ): Unit = {
+
+ val python = pythonBinPath(cuid, pveName).toAbsolutePath.toString
+
+ if (!Files.exists(Paths.get(python))) {
+ queue.put(s"[PVE][ERR] Python executable not found for PVE: $python")
+ return
+ }
+
+ val metadataPath = cuidDir(cuid, pveName).resolve("user-packages.txt")
+
+ var installedPackages =
+ if (Files.exists(metadataPath)) {
+ Files
+ .readAllLines(metadataPath)
+ .asScala
+ .map(_.trim)
+ .filter(_.nonEmpty)
+ .toSet
+ } else {
+ Set[String]()
Review Comment:
This seems like a duplicate of line 216-225 (Different return type but looks
like we can unify the return type as well)
##########
amber/system-requirements-lock.txt:
##########
Review Comment:
This file might need the apache header
##########
amber/src/main/scala/org/apache/texera/web/resource/pythonvirtualenvironment/PveWebsocketResource.scala:
##########
@@ -61,7 +82,6 @@ class PveWebsocketResource {
while (!done && session.isOpen) {
val line = queue.take()
-
Review Comment:
Can you revert this change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
