Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(dev): add node and psql to devcontainer #38

Merged
merged 3 commits into from
Jul 29, 2021
Merged

Conversation

agucova
Copy link
Member

@agucova agucova commented Jul 28, 2021

Para facilitar trabajar con los backups en el devcontainer agregué el cliente de PostgreSQL en la máquina, y cambié las credenciales internas a django reflejando la BBDD real.

También agregué node al contenedor y agregué el npm install al startup. Arregla el bug mencionado en #12

@agucova agucova self-assigned this Jul 28, 2021
@agucova agucova added the bug Something isn't working label Jul 28, 2021
Copy link
Member

@benjavicente benjavicente left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Al construir el contenedor obtengo el error

django.db.utils.OperationalError: FATAL:  role "django" does not exis

esto no evita que el contenedor se construya, aunque obtengo el mismo error al correr python manage.py runserver.

.devcontainer/devcontainer.json Show resolved Hide resolved
.devcontainer/devcontainer.json Outdated Show resolved Hide resolved
@agucova
Copy link
Member Author

agucova commented Jul 28, 2021

django.db.utils.OperationalError: FATAL: role "django" does not exist

Esto es raro, parece ser una regresión de mis tests. Le echaré un ojo.

@agucova
Copy link
Member Author

agucova commented Jul 28, 2021

django.db.utils.OperationalError: FATAL: role "django" does not exist

Esto es raro, parece ser una regresión de mis tests. Le echaré un ojo.

Corrijo, no es una regresión. Simplemente es el resultado de utilizar un devcontainer existente. Incluso si fuerzas su reinstalación por defecto el sistema de archivos de la BBDD es persistente, por lo que este cambio requiere que borren los contenedores antes de clickear "Rebuild container" en la pestaña de Remote Explorer.

tl;dr: hay que borrar la BBDD en sus devcontainers existentes

@benjavicente
Copy link
Member

tl;dr: hay que borrar la BBDD en sus devcontainers existentes

Era eliminar el volumen, ahora me funciona perfecto 🎉

@FarDust
Copy link
Member

FarDust commented Jul 28, 2021

tl;dr: hay que borrar la BBDD en sus devcontainers existentes

Era eliminar el volumen, ahora me funciona perfecto 🎉

Esto hay que agregarlo en la documentación probablemente.

Copy link
Member

@FarDust FarDust left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

En general el código se ve bien y funcional.

.devcontainer/Dockerfile Show resolved Hide resolved
@FarDust
Copy link
Member

FarDust commented Jul 28, 2021

Otra cosa que me llamo la atención, es que el docker-compose para la web app no tiene la clausula depends_on para la base de datos. Esto en teoría puede causar que la base de datos no se inicie antes que la app web.

@nico-mac
Copy link
Member

Los contenedores están fuera de mi conocimiento, pero no veo nada que pueda causar eventuales problemas.

@agucova
Copy link
Member Author

agucova commented Jul 28, 2021

tl;dr: hay que borrar la BBDD en sus devcontainers existentes

Era eliminar el volumen, ahora me funciona perfecto 🎉

Esto hay que agregarlo en la documentación probablemente.

Es una cosa de una vez, no veo por qué documentarlo.

@agucova
Copy link
Member Author

agucova commented Jul 28, 2021

Otra cosa que me llamo la atención, es que el docker-compose para la web app no tiene la clausula depends_on para la base de datos. Esto en teoría puede causar que la base de datos no se inicie antes que la app web.

La plantilla es la oficial para devcontainer, y Django solo puede iniciarse cuando ambos contenedores están completamente listos (porque ambos constituyen el devcontainer), así que no debería ser un problema. Esto si sería relevante si estuviéramos en un sistema de producción donde el contenedor principal lanza Django al inicio.

@agucova agucova merged commit cd53c76 into main Jul 29, 2021
@agucova agucova deleted the fix-devcontainer branch July 29, 2021 04:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants