open-source-uc / bdd Goto Github PK

A pesar de que la Makefile hace prácticamente toda la pega, creo que es importante hacer algo de documentación del setup, que dependencias hay que instalar (poetry, Docker, etc) y cómo configurarse para contribuir (pre-commit, etc).

También se debieran eliminar los archivos del setup antiguo que fueron reemplazados por Docker.

Aquí hay mucha anidación, separaría las iteraciones del for en una función aparte.

Originally posted by @benjavicente in #19 (comment)

Falta aprovechar los docstrings para documentar las cosas. Por ejemplo, no está claro que un curso (course) es un ramo (subject) dictado en un semestre y sección en particular (es decir, el curso es el que uno cursa en un semestre y sección).

si la relación ya está hecha, creo que es innecesario la comparación por where del subject con el school

https://sqlmodel.tiangolo.com/tutorial/connect/read-connected-data/#join-tables-in-sqlmodel

Originally posted by @FarDust in #19 (comment)

Hacer esto va a hacer que el proyecto se vea más atractivo y producido, además así va a ser más fácil difundirlo.

• Configurar ambiente para correr los test
• Definir ambiente de deploy

Se podría agregar:

• Un deploy por cada push a main
• #23
• #24
• #21

Ahora para la mayoría de las cosas es para obtener la lista completa de elementos, lo ideal sería tener algunos endpoints y opciones adicionales como para búsqueda por nombre / código / id y además algunos parámetros para filtrar resultados.

Documentaría este endpoint para señalar porque tiene tantos parámetros.
Supongo que es para imitar los filtros que tiene RamosUC, entonces dejaría ese propósito explícito.

Originally posted by @benjavicente in #19 (comment)

Si este patrón se repite mucho, demás se podría hacer un context para manejarlo:

# ./utils.py
from contextlib import asynccontextmanager

@asynccontextmanager
async def catch_exc(msg: str, *cleanup_fns: list[Callable]):
    try:
        yield
    except Exception:
          log.error(msg, code, exc_info=True)
          errors.add(code)
          for fn in cleanup_fns:
              fn()

# Ejemplos
with catch_exc(f"Cannot get requirements and syllabus for {code}", db_session.rollback):
   ...

No creo que esta versión que deje (y que no he probado) funcione para todos los casos, pero creo que se podría ver para simplificar el código 🤔

Originally posted by @benjavicente in #19 (comment)

Todavía no hay licencia en el repo.

Además, se podrían añadir archivos de utilidad de GitHub, como los templates para PR e issues (idealmente no tan molestos para incentivar la colaboración).

Ahora mkdocs y fastapi están en el mismo pyproject.toml en las dependencias no dev. Esto hace que en producción se instalen dependencias que no son necesarias para la API, y que al crear los docs se instalen cosas de la API que los docs no necesitan.

Una opción para arreglar esto es separar las dependencias en 2 archivos (ej dejar la de los docs en docs/requirements.txt).
Los cambios que se hagan deberían reflejarse en los archivos Dockerfile correspondientes.

Tenemos configurado ya strawberry para GraphQL, ahora falta programar el esquema.

Tiene soporte experimental con pydantic, lo que sería un agrado usar para tener que especificar el esquema 1 vez y que se termine usando en el esquema de postgresql, el de openapi y graphql ✨

Actualmente, cada vez que se inicia la aplicación se crean las todas las tablas mediante:

SQLModel.metadata.create_all(engine)

Esto crea las tablas con el SQL IF NOT EXISTS, es decir, si las tablas ya existen entonces no se modifican para alterar la columnas cuando surgen cambios en los modelos. Debe haber un manejo del estado/versionamiento del schema para permitir hacer cambios cuando sea necesario, sin tener que rehacer la BDD completamente de nuevo.

Probablemente la mejor solución sea usar Alembic (un motor de migraciones para SQLAlchemy). Como referecia dejo esta issue de SQLModel que explica como se puede realizar esta implementación.

Hay que dejar el link a como contribuir al principio del readme, y probablemente halla que añadir una explicación de que es este proyecto, como funciona, y otros links de interés antes de mostrar los comandos de como correrlo.