SarahAsad23 commented on code in PR #5613:
URL: https://github.com/apache/texera/pull/5613#discussion_r3399095291
##########
amber/src/main/scala/org/apache/texera/web/resource/pythonvirtualenvironment/PveManager.scala:
##########
@@ -94,26 +96,95 @@ object PveManager {
}
}
- private def getSystemPath(isLocal: Boolean): Path = {
- Paths.get(
- if (isLocal) "amber/system-requirements-lock.txt"
- else "/tmp/system-requirements-lock.txt"
- )
- }
+ private def locateRequirementsTxt(): Option[Path] =
+ Seq(Paths.get("/tmp", "requirements.txt"), Paths.get("amber",
"requirements.txt"))
+ .find(Files.exists(_))
+
+ // Resolves the fully-pinned system package set by installing
requirements.txt
+ // into a throwaway venv and running `pip freeze`.
+ private def resolveSystemPackages(): Seq[String] = {
+ val requirementsPath = locateRequirementsTxt() match {
+ case Some(p) => p
+ case None =>
+ logger.error("requirements.txt not found; system package set will be
empty")
Review Comment:
We could, but we've consistently referred to these as "system packages" in
previous PVE PRs, and the frontend already uses that terminology when
displaying the requirements. I'd prefer to keep the current naming for
consistency.
--
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]
