SovereignCloudStack / rookify

Enable Ceph-Cluster migrations to Rook
https://scs.community/
Apache License 2.0
1 stars 0 forks source link

feat: adding makefile to install first pre-commit and then venv #29

Closed boekhorstb1 closed 2 months ago

boekhorstb1 commented 2 months ago
brueggemann commented 2 months ago

I did a little bit research. So I think the best solution for the rados library is:

  1. We should remove rados from requirements.txt, as this cannot be installed by pip
  2. We alter the setup-venv target in the Makefile to: i. Use python -m venv --system-site-packages ./.venv to create the venv ii. Do the pip install with the following command: pip install --ignore-installed -r requirements.txt
  3. We do pip freeze -l > requirements.txt to update the requirements.txt with only the local packages
  4. We add some CI/CD job to check if all requirements of the projects are respected in the requirements.txt (This should be another PR)

For explanation: the problem with --system-site-packages is, that as the commandline switch says, it includes the system site-packages to our venv. This can be a problem, because we probably don't have the package versions installed, that are defined in requirements.txt. But this should be compensated with the pip install --ignore-installed command. This ignores the already installed packages, so pip will install the requirements as defined in requirements.txt into the venv. Python will look for local packages first, before it looks in system directories. Another problem of --system-site-packages is, that in pip freeze the system site-packages are listed as well as the locally installed packages. so pip freeze -l mitigates this by only listing the local installed packages.

The only problem this cannot resolve is, that a developer could import a package, that is installed as system package on the development machine and so, don't notice that the requirements.txt has to be updated. But this is a problem we can resolve by checking in a CI/CD pipeline