-
Notifications
You must be signed in to change notification settings - Fork 6
Automate migrations #41
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
Conversation
docker/Dockerfile
Outdated
|
|
||
| RUN mkdir -p /challenge-api/reports | ||
|
|
||
| RUN echo "Running database migrations..." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[performance]
Combining the echo command with the npx prisma migrate deploy command into a single RUN statement can reduce the number of layers in the Docker image, which can improve build performance and reduce image size. Consider using RUN echo "Running database migrations..." && npx prisma migrate deploy.
docker/Dockerfile
Outdated
| RUN mkdir -p /challenge-api/reports | ||
|
|
||
| RUN echo "Running database migrations..." | ||
| RUN npx prisma migrate deploy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[❗❗ correctness]
Running database migrations automatically when the container starts can lead to potential issues if multiple instances of the container are started simultaneously, as they might attempt to run migrations concurrently. Consider implementing a locking mechanism or ensuring that only one instance handles migrations to avoid race conditions.
|
|
||
| CMD ["node","/challenge-api/app.js"] | ||
| # Copy entrypoint script and make it executable | ||
| COPY docker/entrypoint.sh /entrypoint.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[❗❗ correctness]
Ensure that the entrypoint.sh script handles errors gracefully, especially during migrations. This is crucial to prevent the container from entering a crash loop if migrations fail.
|
|
||
| # Start the application | ||
| echo "Starting application server..." | ||
| exec node /challenge-api/app.js |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[💡 design]
Using exec to start the application server is a good practice as it replaces the shell with the node process, ensuring that the process receives signals directly. Ensure that this behavior is intended.
Running migrations automatically when service starts.