fix: pre-commit ci workflow #3917
Conversation
kostasrim
commented
Oct 14, 2024
•
kostasrim
commented
- fix pip install to run under venv
- fix fakeredis python installation
Signed-off-by: kostas <kostas@dragonflydb.io>
.github/workflows/ci.yml
Outdated
| @@ -26,6 +26,17 @@ jobs: | |||
| lsblk -l | |||
| echo "sda rotational = $(cat /sys/block/sda/queue/rotational)" | |||
| echo "sdb rotational = $(cat /sys/block/sdb/queue/rotational)" | |||
| - name: Create virtual environment | |||
There was a problem hiding this comment.
I meant that you need to change the current Install dependencies step line 22 to run in virtual env
There was a problem hiding this comment.
yep yep, I was distracted with the gh runner 😛
.github/workflows/ci.yml
Outdated
| @@ -21,17 +21,26 @@ jobs: | |||
| - uses: actions/setup-python@v5 | |||
| - name: Install dependencies | |||
| run: | | |||
| python -m venv venv | |||
| source venv/bin/activate | |||
| python -m pip install --upgrade pip | |||
There was a problem hiding this comment.
The > python -m pip install --upgrade pip
is not needed. Will remove
.github/workflows/ci.yml
Outdated
| - name: Install dependencies | ||
| run: | | ||
| source venv/bin/activate | ||
| pip install pre-commit |
There was a problem hiding this comment.
because among different run commands you need to source again
There was a problem hiding this comment.
(I am testing this not 100% sure about this)
| @@ -39,18 +36,18 @@ jobs: | |||
| - uses: actions/checkout@v4 | |||
| with: | |||
| submodules: true | |||
| - uses: actions/setup-python@v5 | |||
There was a problem hiding this comment.
For some reason this caused a broken installation of python. I fell back to the one provided by ubuntu (3.10 for 22.04)
There was a problem hiding this comment.
I dont understand we used python 3.10 before in this step
Did you try to remove the cache-dependency-path and see if this worked?
There was a problem hiding this comment.
Yes I think the cache-dependency-path was the problem but I did not try removing it and keeping the actions/setup. We want the the python version that comes with the system and the best way to do that is apt install and not relying on some action to fetch the dependency
.github/workflows/ci.yml
Outdated
| python -m pip freeze --local | ||
| python -m venv venv | ||
| source venv/bin/activate | ||
| pip install pre-commit |
There was a problem hiding this comment.
why not using python -m?
why did you remove python -m pip freeze --local
There was a problem hiding this comment.
- Does it really matter if I use
python -morpip installdirectly ? - Because it's irrelevant -- it freezes the current state of the virtual env such that it can be recreated somewhere else. We dont't need that here
|
This #3924 fixes the problem. Really no need for all this virtual env stuff in our CI |
---break-system-packages Can break the system |
