SarahAsad23 commented on code in PR #5613:
URL: https://github.com/apache/texera/pull/5613#discussion_r3399294940
##########
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")
+ return Seq.empty
+ }
- def getSystemPackages(isLocal: Boolean): Seq[String] = {
- if (!Files.exists(getSystemPath(isLocal))) {
- Seq()
- } else {
- Files
- .readAllLines(getSystemPath(isLocal))
- .asScala
- .map(_.trim)
- .filter(line => line.nonEmpty && !line.startsWith("#"))
- .toSeq
+ val tempVenv = Files.createTempDirectory("texera-system-venv-")
+ try {
+ val python = tempVenv.resolve("bin").resolve("python").toString
+ val createCode =
+ Process(Seq(PythonUtils.getPythonExecutable, "-m", "venv",
tempVenv.toString)).!
+ if (createCode != 0) {
+ logger.error(s"failed to create temp venv for system-package
resolution (exit=$createCode)")
+ return Seq.empty
+ }
+
+ val installCode = Process(
+ Seq(
+ python,
Review Comment:
The Process(Seq(python, ...)) invocations themselves are cross-platform as
they bypass the shell and Python handles platform differences internally. The
only Windows incompatible bit is the venv internal path: bin/python (POSIX) vs
Scripts/python.exe (Windows). I'll factor that into a small helper rather than
splitting into separate .sh/.bat/.ps1 scripts as the scripts would be 90%
identical and the OS check is one line in Scala.
--
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]
