-
Notifications
You must be signed in to change notification settings - Fork 57
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
Support workdir venv #1548
base: main
Are you sure you want to change the base?
Support workdir venv #1548
Conversation
Needs rebase after main changed. |
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.
Needs a rebase but other than that I agree with the change.
Should be ready, but lets wait for the build: |
When using CI, we want to use a fresh venv for every build so bad venv created by one job does not break the next job, or bad change in a PR is not hidden by good venv created by a previous job. When using local development, you may want to have venv per ramen source directory. One case is using git worktree to have multiple ramen trees. With this change you can create a venv per ramen worktree. Example usage: hack/make-venv .venv/ramen Since this is the expected usage, add this path to gitignore, so using `git clean dxf` does not remove the directory. Signed-off-by: Nir Soffer <[email protected]>
Create venv inside the ramen checkout directory to have fresh venv for every build. Since drenv is not installed on the runner, we need to enter the venv in any step script. I hope we can find a way to enter the venv once for the entire job, but this is good enough for now. Signed-off-by: Nir Soffer <[email protected]>
@nirs Do you think the e2e failure is related? |
No, this is a known issue with cephfs. Sometimes we fail to clean up after the volsync file test, since the cpehfs snapshot is not deleted. When this happens, retrying the e2e job will continue to fail because we are using the same namespaces and resources names, so stuck snapshot will continue to fail without manual cleanup. If there was an issue with the venv, deleting the cluster would fail and we would not reach the step of starting the clusters. |
The next failure: https://github.com/RamenDR/ramen/actions/runs/11955746683/job/33336392815 |
@raghavendra-talur this is ready for merge, the random failures in the ci should be fixed by #1689 |
Support creating venv per workdir:
With this you can test drenv changes in the ramen-foobar worktree without affecting the main worktree, using the standard venv (~/.venv/ramen).
Use this in the CI to ensure that every build uses a fresh venv instead of installing drenv on the runner host.