SarahAsad23 commented on code in PR #5613:
URL: https://github.com/apache/texera/pull/5613#discussion_r3399147335
##########
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-")
Review Comment:
I'd prefer to keep the current naming for consistency with the rest of the
PVE implementation, where we refer to these as system packages/requirements.
--
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]