- Notifications
You must be signed in to change notification settings - Fork6
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
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 theecho command with thenpx prisma migrate deploy command into a singleRUN statement can reduce the number of layers in the Docker image, which can improve build performance and reduce image size. Consider usingRUN 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 theentrypoint.sh script handles errors gracefully, especially during migrations. This is crucial to prevent the container from entering a crash loop if migrations fail.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| # 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]
Usingexec 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.
b829e34 intodevelopUh oh!
There was an error while loading.Please reload this page.
Running migrations automatically when service starts.