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

Build and dev refactoring #1941

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

Panikabor
Copy link
Contributor

Rework of development environment and docker image build.

This PR includes all the work done by @BboyKeen on his PR (#1769)

See HOWTO-DEV.md to understand how the dev environment should now be setup and run.

The changes eliminates py3compile, the need for a full compilation env in the docker image and on the developer's machine. It keeps the Makefile as a shortcut to run some tasks in dev, but does not make it mandatory.

Most of the changes work both in Windows and in Linux, I could not test on MacOS because I do not have a Mac. On Windows, there are still some rough edges, but it is nearly working as well as in Linux.

List of changes :

  • Do not execute the code inside a docker container in the dev environment, but prefer the use of a virtual env instead
  • Use an official debian slim python image instead of a debian in which we install python
  • Removed most of the dependencies from the Dockerfile
  • Modified setup.py/requirements to make installation in dev environment easier
  • Use setup.py to install the API in the docker image
  • add init.py files where they were missing
  • Removed the need of installing postgres on local dev environment (by switching to psycopg2-binary instead of psycopg2)
  • Kept the Makefile as a shortcut for tasks to be run in the dev environment
  • Make env_replace script compatible with Windows
  • Rewrite documentation on how to setup the dev environment (can be put in the github wiki in the end instead of the HOWTO-DEV.md file, but I did not want to modify the wiki before the PR is merged)

I could not test the production dockerfile, but it is nearly the same as the Dockerfile.dev (which in fact should not be necessary anymore), so it should be working well.

Kévin Liagre and others added 9 commits March 20, 2024 16:41
…EV.md to understand how the dev environment should now be setup and run.

The changes eliminate py3compile, the need for a full compilation env in the docker image and on the developer's machine. It keeps the Makefile as a shortcut to run some tasks in dev, but does not make it mandatory.
@Panikabor
Copy link
Contributor Author

Just looked at the Codacy report, it has only false positives. I cannot look at snyk reports, as I have no rights to look at them. I requested the access, but I suspect tah it will be about known vulnerabilities in packages that are already used by current version of the code, which I did not change.

@Panikabor
Copy link
Contributor Author

Looks like fixing the merge conflict made snyk happy. I'm looking at the test failures.

## Requirements

- A computer running Linux or Windows (on Windows, take a look at [WSL](https://learn.microsoft.com/fr-fr/windows/wsl/install), it will make your developer life easier). Maybe MacOs can work, but it was not tested.
- Python 3.9.x installed with pip and the virtualenv package (currently, it will not work with python 3.10+)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what ? Current Ubuntu LTS 24.04 has 3.11, it will not work ? why ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some dependencies (at least cornice in the version you are using) are not compatible with python 3.10+. I tried to update it to the latest version, but it did not work and I fear that if we try to update it then it will be another.

- Python 3.9.x installed with pip and the virtualenv package (currently, it will not work with python 3.10+)
- On Ubuntu, you can install it from the deadsnakes repository if your embedded python version is too recent (https://askubuntu.com/questions/1318846/how-do-i-install-python-3-9)
- On Windows, use Microsoft Store to install it (https://apps.microsoft.com/detail/9p7qfqmjrfp7)
- A docker environment with docker compose plugin (either [docker desktop](https://www.docker.com/products/docker-desktop/) or a manual installation)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is podman compatible ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say yes, but I did not test. I made my tests in WSL using a manually installed docker (as docker desktop is now a paid application if you want to use it in a profesionnal context)

@Panikabor
Copy link
Contributor Author

Tests are now running, but some of them are failing, which is not the case locally. I'll try to have a look tomorrow, but it looks strange as my PR does not change any code.

@eddy-geek
Copy link

eddy-geek commented Jan 29, 2025

I tested this procedure on existing ubuntu 24.04 (heavily customized).
I don't remember before, but it seems good overall :-)

some side notes specific to my system

  • I had to remove the old docker that does not have docker compose subcommand, and the existing native redis that was taking the port of the docker: sudo apt remove docker.io redis
  • I had to run venv/bin/python -m pytest to avoid the global anaconda env site-packes leaking in

the "requirements" part boils down to

sudo apt install podman podman-docker docker-compose
sudo add-apt-repository ppa:deadsnakes/ppa
sudo apt install python3.9 python3.9-venv

Some tests fail though, they all seem to come from the syncer:

...
c2corg_api/tests/views/__init__.py:92: in assertNotifiedEs
E   AssertionError: unexpectedly None : no sync. notification sent for ES
...
===================== 33 failed, 823 passed, 19 warnings in 337.50s (0:05:37) =====================

the logs point to disk issues but unclear if tht's really it

> docker logs -f c2c_v6_api-elasticsearch-1
[2025-01-29 21:39:55,313][INFO ][cluster.routing.allocation.decider] [Brunnhilda]
  rerouting shards: [high disk watermark exceeded on one or more nodes]

> docker compose exec -T elasticsearch df -h
Filesystem      Size  Used Avail Use% Mounted on
overlay         468G  427G   18G  97% /
tmpfs            64M     0   64M   0% /dev
shm              63M     0   63M   0% /dev/shm
tmpfs           3.2G  2.9M  3.2G   1% /etc/hosts
/dev/nvme0n1p2  468G  427G   18G  97% /usr/share/elasticsearch/data

@Panikabor
Copy link
Contributor Author

For your problem with tests, do you still have a docker container with the API running, that could be consuming the test events in place of the tests syncer ?

@Panikabor
Copy link
Contributor Author

Tests are fixed in CI

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.

2 participants