climatepolicyradar / policy-search

0 stars 1 forks source link

Overall project structure #142

Closed chrisaballard closed 2 years ago

chrisaballard commented 2 years ago

Define a structure for the codebase:

chrisaballard commented 2 years ago

Review the following open source repos to get a handle on the best options out there.

rubrix

Rubrix has a python backend with Vue frontend. Both are in the same repo along with docker-compose.yml:

- /frontend - vue frontend code
- /src/rubrix - python backend code
  - /client - python client api
  - /server - fastapi
- /tests - backend unit tests using pytest

I can't find any unit tests for front end.

dagster

Dagster has a python backend with a React frontend. Frontend and backend are in the same repo.

Uses make and a corresponding Makefile to handle setting up dev environments, installing dependencies (e.g. building the dagit frontend) and running tests.

Interestingly uses a python script to install python packages. Uses a makefile and the linux make command to install and build dependencies - e.g. a dev environment:

make dev_install

Uses yarn package manager for installing and building the frontend. This also has commands for running front end unit tests - e.g. yarn test which will run tests affected by code changes only.

- /jsmodules/dagit - frontend
- /python_modules - python backend code
  - .../dagster - source for module
     - .../dagster-test - pytest unit tests for module
- /scripts - main scripts for install
  - /packages/app - react front end code
  - package.json - yarn 
  - Makefile - makefile for frontend
- Makefile - instructions for building the app

LabelStudio

Python/Django backend with two React frontends. the main label-sudio repo includes the frontend and the backend,

There is also a separate frontend repo which includes labelling components. This is because a big use case for them is to allow frontend components to be integrated into other apps, so it makes sense to do it this way. Haven't quite got my head round how this relates to the front end in the main app.

Frontend and backend code is merged into a single source structure, not separated as with our current app.

- /label_studio
  - /frontend/src - front end react app
  - /core - core server-side python code
  - server.py - django server entry point
- /scripts
- Dockerfile
- docker-compose.yml
- Makefile
kdutia commented 2 years ago

@chrisaballard some more examples of repos:

Cookiecutter (in second example) is a super useful tool if we ever want to look into how to manage reproducible data science research in github.

chrisaballard commented 2 years ago

Thanks for the links @kalyan. I've come across the cookiecutter template for fastapi/react app before, and I think this is probably a great option for us. Will look into this more closely.

@chrisaballard some more examples of repos:

* https://github.com/heartexlabs/label-studio

* https://github.com/Buuntu/fastapi-react

Cookiecutter (in second example) is a super useful tool if we ever want to look into how to manage reproducible data science research in github.

chrisaballard commented 2 years ago

I have created a new repository for navigator at https://github.com/climatepolicyradar/navigator. This is based on the fastapi-react cookiecutter template but has been modified for our purposes.

The structure separates the python backend and js frontend into a single repository with the following top level structure:

- backend
- frontend
- nginx
- scripts
- docker-compose.yml
- Makefile
- ...

Out of the box it provides:

There are also other helpful goodies documented in https://github.com/Buuntu/fastapi-react

Local development can be carried out using docker or the local machine. A Makefile is provided to easily set up either dev environment.

If using docker running make build will run the docker containers and db migrations. For local development, running make dev_install will install a local poetry environment and install git precommit hooks. See README.md for more details.

chrisaballard commented 2 years ago

Assigned to @kdutia to double check that make dev_install works ok on MacOS.

kdutia commented 2 years ago

@chrisaballard make dev_install is failing for me on the make poetry_environment step. If I set cd backend/; poetry config virtualenvs.create true, then poetry installation works (although installation of a later package fails, but we'll get to that).

Wondering what your reasoning was for setting that flag to false? The reason I did it for the Dockerfile in the prototype is because we don't need to spawn virtualenvs within Docker, but as far as I understand I'd want Poetry to create its own virtualenv on my local machine. (Although I think I might then have issues installing Pytorch later down the line via Poetry, yay M1!).

This is the later package installation failure. I'll look into it. image.png

kdutia commented 2 years ago

@chrisaballard update: doing that replacement meant I could do a poetry install ok, and pre-commit hooks successfully install inside my poetry shell. See this commit

kdutia commented 2 years ago

@chrisaballard I created a PR because I realised it'd be slightly easier for you to review :) https://github.com/climatepolicyradar/navigator/pull/1

eurolife commented 2 years ago

@chrisaballard I'm unable to complete the build of the container, as I am getting this error. I don't know if it has something to do with the config of DATABASE_URL which I was unable to find set anywhere in the project. If I need to set it somewhere, please let me know where (in a .env file?) and what it is supposed to be.

Screenshot 2021-12-14 at 17 45 55

chrisaballard commented 2 years ago

chrisaballard make dev_install is failing for me on the make poetry_environment step. If I set cd backend/; poetry config virtualenvs.create true, then poetry installation works (although installation of a later package fails, but we'll get to that). Wondering what your reasoning was for setting that flag to false?

@kdutia there was no reason - I simply copied the code from the Dockerfile, but now I understand the reason why you created it that way originally we obviously need to change it!

I'll review your PR and will update the Makefile to remove the line which disables the creation of the virtual environment:

poetry_environment:
    cd backend/; poetry install
chrisaballard commented 2 years ago

Merged pr https://github.com/climatepolicyradar/navigator/pull/1 into main.

chrisaballard commented 2 years ago

@chrisaballard I'm unable to complete the build of the container, as I am getting this error. I don't know if it has something to do with the config of DATABASE_URL which I was unable to find set anywhere in the project. If I need to set it somewhere, please let me know where (in a .env file?) and what it is supposed to be.

Screenshot 2021-12-14 at 17 45 55

@eurolife just looking at the problem you had running the containers. One thing I neglected to document in the readme is that you need to create these environment variables. The best way to do this is to create a .env file. I have added the following to the repo:

To get things running you need to:

If you open the project in vscode, it should detect that the project is setup to use a devcontainer. Note that this is preconfigured to start in the backend container. To use the frontend for dev you need to update the devcontainer.json file as described in the readme. Hopefully this should sort out your issues, but let me know if you're still having problems.

eurolife commented 2 years ago

@chrisaballard If you saw my previous response, please disregard, I figured out the issue, it is working now.

eurolife commented 2 years ago
  • @eurolife can you please review the frontend structure and consider how nextjs would fit into this structure? I'm not wedded to the react admin frontend, but it seemed sensible to leave it in due to the user admin support which will be helpful before we write something more suited to our needs.

@chrisaballard I'm looking at migrating the existing frontend codebase into NextJS. While I am sure it is possible, it's looking to be a bit tricky / messy and may take more time than just starting with an empty frontend directory and setting up a fresh Nextjs framework in that, and copying over what is needed for the user admin when we need it. I'm only thinking this way because I feel I need to get a start on the form to submit a policy and only have about a day or day and a couple hours to do it. So I think to get that form done quickly for now, we have 2 options:

chrisaballard commented 2 years ago

Think best option is no. 2

kdutia commented 2 years ago

@chrisaballard @eurolife I've merged a PR which gets next.js working for us inside docker and docker-compose. Are we ok to close this issue?

eurolife commented 2 years ago

Ok with me from a front end perspective