andreagrandi/covid-api

Directory structure

MatMoore opened this issue · 4 comments

Hi, I thought I'd create a ticket to discuss the proposed structure as you mentioned you are still finalising it.

Currently we have this

covidapi/
├── api
│   ├── __init__.py
│   └── main.py
└── db
    ├── __init__.py
    ├── crud.py
    ├── database.py
    ├── models.py
    └── schemas.py

This is based on the FastAPI recommendation of

└── sql_app
    ├── __init__.py
    ├── crud.py
    ├── database.py
    ├── main.py
    ├── models.py
    └── schemas.py

The intention is to add an app.py to the root (see #6) and then put view modules under api/.

This separation of API and db seems logical to me, but I think that the schemas.py does not belong there because it defines the pydantic model which is not actually required for the db part.

crud.py also feels like a higher level thing as it translates between the two kinds of models. What do you think about moving these two modules?

If we were to separate things as much as possible, the directory structure could look like this:

covidapi/
├── db             # only depends on SQLAlchemy (and alembic?)
│ ├── __init__.py
│ ├── database.py
│ └── models.py
├── schemas
│ ├── __init__.py
│ └── schemas.py   # pydantic models
├──services        # could contain other things used by the views
│ └── crud.py      # accepts/returns pydantic models, encapsulates the db
└── views          # uses services and schemas
   ├── __init__.py
   └── main.py 

Alternatively schemas and crud.py could be moved into the api layer.

What do you think?

Hi Mat 👋 thanks for the Issue and the PR. I will try to have a look at both this evening.

So, I'm generally ok with the structure you propose, but I have a couple of doubts:

  • why main.py under views/ ?
  • I would place crud.py under db/, unless I'm missing something else

I agree with separating pydantic schemas/ and db/ models

main.py in views is just meant to be the placeholder file that's there at the moment. I might have misunderstood what that was a placeholder for though. I'm assuming it will be replaced by individual modules for the different endpoints.

Keeping crud.py in db is another option. I think the downside of that is that it imports schemas.py, so if you don't move that as well, the db package would still depend on the schemas package, rather than the two being completely independent of each other. Also, if the views are only going to call crud.py then separating that out means you can modify the views without thinking about the db package at all, and vice versa.

That said, either way is probably fine - it's quite a small project so it should be relatively easy for contributors to understand the existing code.

the db package would still depend on the schemas package, rather than the two being completely independent of each other

I see your point now! I agree with you, let's use the folder structure you propose. Feel free to prepare a PR or I can try to do it this evening, thanks!