-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Fix] Move Python venv folder location to ~/.velocitas/packages/ #336
base: main
Are you sure you want to change the base?
Conversation
@@ -135,7 +135,7 @@ Other Python processes spawned from a Python-based program will **not** be autom | |||
(Because the dependencies of that scripts will probably differ from those of the calling program's component.) | |||
|
|||
The venv of each component is created within the project cache directory in the folder `pyvenv`. For example the | |||
component `Foo`s venv would be located in `/home/vscode/.velocitas/projects/<hash>/pyvenv/foo/`. This also contains | |||
component `Foo`s venv would be located in `/home/vscode/.velocitas/packages/.pyvenvs/foo/`. This also contains |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should better do a minor update to the line 137 to use the right folder name
"package cache directory in the folder .pyvenvs
."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks in general ok, there are some minor editorial changes that could be updated.
Are there some scenarios where we theoretically could run into problems by changing from "unique" paths to common paths. Like if having multiple projects with different python dependencies in the same workspace7devcontainer?
This fix will not go into R2 eng drop. Furthermore, it will only fix the issue with the CLI component`s venvs. We also have an issue with the cached vspec file and model. So, lets do one step backwards and rethink for an overall solution. |
@@ -151,7 +152,7 @@ async function createPythonVenv( | |||
cwd: string, | |||
loggingOptions: { writeStdout?: boolean; verbose?: boolean }, | |||
) { | |||
const venvDir = `${ProjectCache.getCacheDir()}/pyvenv/${componentId}`; | |||
const venvDir = `${getPackageFolderPath()}/.pyvenvs/${componentId}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixes just the Python venv issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would argue this is not a package and should not go in the package path.
My suggestion would be to move all project related data which is right now put into $VELOCITAS_HOME/projects/<HASH>
directly into the project's workspace under a hidden folder .velocitas
that way the user is able to freely rename or move his project and still keep all of his cache and project data intact.
This would even be an easy fix by introducing and environment variable called VELOCITAS_INLINE_PROJECT
which, if set, redirects the output which would usually go to $VELOCITAS_HOME/projects/<HASH>
to $VELOCITAS_WORKSPACE/.velocitas
Since CLI v0.13.0 the Python dependencies of each CLI component (which is using Python) is maintained in the Velocitas cache under
~/.velocitas/projects/<md5-hash>/pyvenv/
.The problem arising with that path is that the md5-hash depends on the name of the workspace folder. In case of the offline container this can lead to the following issue:
During creation with the Lattice app template the folder is
/workspaces/vehicle-app-cpp-template-lattice
. If the receiver of the offline container puts it into a folder named differently than<...>/vehicle-app-cpp-template-lattice
, e.g.<...>/my_app
then the folder inside the running devcontainer is/workspaces/my_app/
resulting in another hash calculated for the Velocitas cache. All the Python venvs initialized during the offline container creation are then located in the wrong cache folder. A re-initialization of the venvs would require an online connection, which is not supposed to be available with the offline container.This fix moves the venv folders to a different location being independent on the workspace folder name:
The new path is
~/.velocitas/packages/.pyenvs/
.