opencdms-dev / legacy-opencdms-api

⭐🐍 OpenCDMS server application written in Python (FastAPI) and exposing a web interface for `opencdms-app` and other applications
MIT License
3 stars 3 forks source link

Feat/integrate surface mch #7

Closed Shaibujnr closed 2 years ago

Shaibujnr commented 3 years ago

This PR mounts the django surface project and mch_api flask wsgi applications on opencdms_server's FastAPI application using the WSGIMiddleware and WSGIAuthmiddleware. The WSGIAuthMiddleware prevents unauthenticated access to both the surface and mch apis.

The surface app is mounted on path /surface while the mch app is mounted on path /mch.

Requirements

Dev

  1. Activate virtual environment
  2. make install
  3. make surface_db_upgrade (To run the surface db migration)
  4. make surface_db_load_initial_data (To load the surface db with initial data)
  5. make serve (To start development server)
  6. visit localhost:5070/docs for swagger api documentation
  7. Surface api is mounted on path localhost:5070/surface/*
  8. Mch api is mounted on path localhost:5070/mch/*

NOTE In docker-compose.yml , opencdms_api service mounts the code directory into the container. Therefore the work directory in the container will match your code structure. This means, if you don't have the surface project cloned, it would be removed from the container once your volume is mounted. For the surface api to work, with volume mounting, run git clone https://github.com/Shaibujnr/surface.git surface. This will create a surface folder and clone the surface django project into it.

Alternatively: comment out the volume mounting (This will prevent reload on code change) and the cloned surface project from the image would still be in the workdir.

faysal-ishtiaq commented 3 years ago

I reviewed the PR as @isedwards asked me to and the code looks really great. And here is my observation:

for opencdms_surface_db


    volumes:
      - ./:/code
      - ./data/shared:/data/shared
      - opencdms_surface_data:/var/lib/postgresql/data/

I think what he tried to achieve is, when there is any change in the code (while developing), we don't need to rebuild the docker image and restart to reflect those changes. If that is the case, I would like to recommend adding specific volume for specific directory for this. Such as -

for opencdms_api service

    volumes:
      - src:/code/src

for opencdms_surface_db


    volumes:
      - ./:/code # WE DON'T NEED THIS HERE
      - data/shared:/data/shared
      - opencdms_surface_data:/var/lib/postgresql/data/

I also noticed that we are using docker-compose version 3.8 which is only available for docker desktop and not for edge service. So, probably going with "3" will be more suitable.

And to start the uvicorn server for fast api, probably we should remove this form docker-compose:


    command:
      [
        "uvicorn",
        "opencdms_server.main:app",
        "--host",
        "0.0.0.0",
        "--port",
        "5000",
        "--reload",
        "--use-colors",
      ]

and add it as entrypoint in dockerfile so that the app runs when the container is started whether on local machine using docker-compose or on server using docker run command.

Also, from my past experience, the following directory structure works great for fastapi projects in the long run

- models 
- - user_model.py
- - other_model.py
- schemas
- - user_schema.py
- - other_schema.py
- controllers
- - auth_controller.py
- - other_controller.py
- services
- - auth_service.py
- - other_service.py
- tests
- - integration
- - unit
- 

Also, I got this error when I ran make surface_db_upgrade

ο‚›  ξ˜† venv ~/PycharmProjects/opencdms-server-shaibujnr on ξ‚  feat/integrate-surface-mch β€’1 βŒ€1 βœ— ❯ make surface_db_upgrade 
docker-compose run opencdms_api python surface/api/manage.py migrate
Starting opencdms_mch_db     ... done
Starting opencdms_surface_db ... done
Traceback (most recent call last):
  File "/code/surface/api/manage.py", line 22, in <module>
    execute_from_command_line(sys.argv)
  File "/usr/local/lib/python3.9/site-packages/django/core/management/__init__.py", line 419, in execute_from_command_line
    utility.execute()
  File "/usr/local/lib/python3.9/site-packages/django/core/management/__init__.py", line 363, in execute
    settings.INSTALLED_APPS
  File "/usr/local/lib/python3.9/site-packages/django/conf/__init__.py", line 82, in __getattr__
    self._setup(name)
  File "/usr/local/lib/python3.9/site-packages/django/conf/__init__.py", line 69, in _setup
    self._wrapped = Settings(settings_module)
  File "/usr/local/lib/python3.9/site-packages/django/conf/__init__.py", line 170, in __init__
    mod = importlib.import_module(self.SETTINGS_MODULE)
  File "/usr/local/lib/python3.9/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1030, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
  File "<frozen importlib._bootstrap>", line 972, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
  File "<frozen importlib._bootstrap>", line 1030, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
  File "<frozen importlib._bootstrap>", line 986, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 680, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 850, in exec_module
  File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
  File "/code/surface/api/tempestas_api/__init__.py", line 5, in <module>
    from .celery import app as celery_app
  File "/code/surface/api/tempestas_api/celery.py", line 14, in <module>
    app.conf.broker_url = settings.SURFACE_BROKER_URL
  File "/usr/local/lib/python3.9/site-packages/django/conf/__init__.py", line 82, in __getattr__
    self._setup(name)
  File "/usr/local/lib/python3.9/site-packages/django/conf/__init__.py", line 69, in _setup
    self._wrapped = Settings(settings_module)
  File "/usr/local/lib/python3.9/site-packages/django/conf/__init__.py", line 170, in __init__
    mod = importlib.import_module(self.SETTINGS_MODULE)
  File "/usr/local/lib/python3.9/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "/code/surface/api/tempestas_api/settings.py", line 227, in <module>
    ENTL_PRIMARY_SERVER_PORT = int(os.getenv('ENTL_PRIMARY_SERVER_PORT'))
ValueError: invalid literal for int() with base 10: ''
make: *** [makefile:2: surface_db_upgrade] Error 1
Shaibujnr commented 3 years ago

Hello @faysal-ishtiaq

Thank you for the detailed feedback. I believe my instructions in the readme file are not complete. I'll update that.

You are correct, the first error you got

python: can't open file '/code/surface/api/manage.py': [Errno 2] No such file or directory
make: *** [makefile:2: surface_db_upgrade] Error 2

is as a result of the volume mounting. As I had explained in the PR, if you don't already have the surface project cloned, it won't be present in the container as well. So to get it to work, you either clone the surface project into the surface directory or comment out the volume mounting configuration in the docker-compose file.

I went with the first approach, which brings us to the second point you made, I totally agree with you, the volume mounting can be more efficient and we definitely should only mount the src directory. I only mounted the entire thing because I was also making changes to the surface project to test and debug.

Moving the uvicorn command to the dockerfile and making it an entrypoint command is also a great idea. I'd do that.

Lastly, I believe the last error is due to the absence of the .env file required by docker-compose to populate the environment variables in the container. for every *.example.env create the corresponding *.env file and just copy the contents of the example env to the env file.

isedwards commented 2 years ago

@Shaibujnr could you try the version of MCHtablasycampos.def here: https://github.com/opencdms/opencdms-test-data/tree/main/schemas/mch

faysal-ishtiaq commented 2 years ago

Hi @Shaibujnr ,

I have tried to run the project again on my localhost. I have gone through all the steps that you have mentioned in the Readme file. However, after running make install when I tried to run make surface_db_upgrade, I faced an error. Then I tried docker-compose build. Then tried make surface_db_upgradeagain. It threw another error. Then I tried docker-compose up -d --build and tried make surface_db_upgrade again. This time there is this line printed on the console.

docker-compose exec opencdms_api python surface/api/manage.py migrate

Then this error

Traceback (most recent call last):
  File "/code/surface/api/manage.py", line 22, in <module>
    execute_from_command_line(sys.argv)
  File "/usr/local/lib/python3.9/site-packages/django/core/management/__init__.py", line 419, in execute_from_command_line
    utility.execute()
  File "/usr/local/lib/python3.9/site-packages/django/core/management/__init__.py", line 363, in execute
    settings.INSTALLED_APPS
  File "/usr/local/lib/python3.9/site-packages/django/conf/__init__.py", line 82, in __getattr__
    self._setup(name)
  File "/usr/local/lib/python3.9/site-packages/django/conf/__init__.py", line 69, in _setup
    self._wrapped = Settings(settings_module)
  File "/usr/local/lib/python3.9/site-packages/django/conf/__init__.py", line 170, in __init__
    mod = importlib.import_module(self.SETTINGS_MODULE)
  File "/usr/local/lib/python3.9/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1030, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
  File "<frozen importlib._bootstrap>", line 986, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 680, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 850, in exec_module
  File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
  File "/code/surface/api/tempestas_api/settings.py", line 227, in <module>
    ENTL_PRIMARY_SERVER_PORT = int(os.getenv('ENTL_PRIMARY_SERVER_PORT'))
ValueError: invalid literal for int() with base 10: ''
make: *** [makefile:2: surface_db_upgrade] Error 1

Please advise me what to do next.

isedwards commented 2 years ago

Hi @faysal-ishtiaq - on your own test branch can you make whatever changes you need to achieve the following:

Shaibujnr commented 2 years ago

Hello @faysal-ishtiaq did you set up the env files correctly? Can I see the contents of your env files?