Skip to content
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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

BjoernAtBosch
Copy link
Member

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/.

@@ -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
Copy link
Contributor

@erikbosch erikbosch Oct 7, 2024

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."

Copy link
Contributor

@erikbosch erikbosch left a 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?

@BjoernAtBosch
Copy link
Member Author

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}`;
Copy link
Member Author

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.

Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants