Repository navigation
fix: rotar logs/atlas.log y reemplazar print() por logger - #52
ranto-dev05 wants to merge 3 commits into
Conversation
Axel-DaMage
left a comment
There was a problem hiding this comment.
Solicito cambios antes de aprobar. El PR mezcla 3 concerns distintos y tiene varios bugs bloqueantes.
Bloqueantes
1. Alcance: el PR mezcla 3 issues no relacionados
El título y el body dicen Close #45 (rotación de logs), pero el PR incluye:
- Docs nuevas (CHANGELOG.md, DEVELOPMENT_GUIDE.md, RELEASE_NOTES.md) — sin issue.
- Dashboard FastAPI +
list_events()(commitfeat: add web dashboard (FastAPI) - issue #42).
Esto rompe trazabilidad y revertibilidad. Pediría partir en 3 PRs:
- PR A: solo rotación de logs + reemplazo de
print()(cierra #45). - PR B: solo infra de docs (con su propio issue).
- PR C: dashboard (cierra #42), con el dashboard cableado (ver punto 2).
2. El dashboard es dead code (no cierra #42)
bot/web/app.py define la app FastAPI pero nada la arranca: no hay uvicorn.run(...), no se integra con main.py ni con el ciclo de vida del bot. El endpoint /api/eventos es inalcanzable en runtime. #42 queda sin cerrar. Falta el runner (hilo/asyncio task que levante uvicorn junto al bot, o un entrypoint separado).
3. XSS en app.js
card.innerHTML = `...${event.name || 'Sin nombre'}...`;event.name viene de eventos programados de Discord (contenido controlado por usuarios). Interpolarlo en innerHTML es XSS. Usar textContent o escapar HTML antes de inyectar.
4. Bug null en app.js
const statusClass = `status-${event.status.replace('-', '').toLowerCase() || 'scheduled'}`;Si event.status es null/undefined, .replace lanza TypeError. El fallback || 'scheduled' se aplica al resultado de toLowerCase(), no a event.status. Corregir:
const status = (event.status || 'scheduled').replace('-', '').toLowerCase();
const statusClass = `status-${status}`;5. Pérdida de stdout
configure_logging() solo registra RotatingFileHandler (archivo). Los print() eliminados iban a consola. En contenedores/CI el stdout es el estándar; sin un StreamHandler se pierde toda salida visible en runtime. Añadir un StreamHandler además del file handler.
Importantes
6. requirements.txt sigue en UTF-16 LE con CRLF
Es preexistente en main, pero este PR lo modifica (añade fastapi/uvicorn) sin normalizar la codificación. pip install -r puede fallar en entornos que no toleran BOM UTF-16. Aprovechar el PR para pasarlo a UTF-8 LF.
7. DEVELOPMENT_GUIDE.md: typo de idioma
Sección "Convenções de Código" — "Convenções" es portugués. Debe ser "Convenciones".
Menores
- Los 3
.mdnuevos no tienen newline final (\ No newline at end of file). list_events()y el endpoint/api/eventosno tienen tests.- El body del PR no referencia
#42(solo el commit), así que GitHub no lo cerrará automáticamente aunque se mergee.
Lo que sí está bien
RotatingFileHandlercon 5 MB / 5 backups: correcto y resuelve #45.- Reemplazo de
print()porloggercon formato lazy (%s): buena práctica. os.makedirs(LOG_DIR, exist_ok=True): cubre clones limpios.- No se expone el token (se loguea
bool(...), no el valor).
Close #45
Contexto
logs/atlas.logcrece sin límite porquelogging.basicConfigescribe en un archivo fijo sin rotación, y variosprint()de depuración se mezclan con el logging.Cambios
main.py: se reemplazalogging.basicConfig(filename=...)porRotatingFileHandler(5 MB, 5 backups) medianteconfigure_logging(), que además garantiza la existencia delogs/en clones limpios.main.pyybot/cogs/events.py: todos losprint()se reemplazan porlogger.info/logger.warning/logger.error, eliminando los mensajes duplicados que ya emitíalogger.Verificación
python -m compileall -q main.py botOKruff check .OKbandit -q -r bot main.pyOKatlas.log,atlas.log.1,atlas.log.2