-
Notifications
You must be signed in to change notification settings - Fork 22
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
automated migrations #848
automated migrations #848
Conversation
this commit: - moves the sql migration scripts into the `database` image - adds a new database named `migrations` to track the list of applied migrations - overrides the entrypoint of the `database` image to have the migrations applied on startup and controlled with the SHANOIR_MIGRATION environment variable - renames the migrations scripts to have them orderd by a sequence number (rather than a timestamp)
5ecc5f2
to
3a54189
Compare
3a54189
to
f5e9567
Compare
df4a07e
to
76531da
Compare
I have a little question: Otherwise it works quite well, maybe put a default value for SHANOIR_MIGRATION=auto in variable.env ? |
# - migration are applied in alphebetical order of the filename ("<SCRIPT>.sql") | ||
# | ||
# - on container startup the behaviour is selected by the SHANOIR_MIGRATION | ||
# environment variable: |
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.
Set auto as default on in docker-compose/database/variable.env ?
This would interfere with hibernate. case "$SHANOIR_MIGRATION" in
auto)
env_setdefault spring.datasource.initialize false
env_setdefault spring.jpa.hibernate.ddl-auto update Perhaps we can rename the current behaviour of Now the question is: how should we report errors. Should we abort the container at startup (fail fast) or just ignore the errors. (I would prefer the first one).
it is already set to |
I agree with you, the database container deployment should fail, to prevent any further database-microservice errors
Yep but it's not taken into account on my local environnement => If nothing is set in variable.env, it fails. |
minor modif by Anthony, and then we accept |
The 'dev' mode implements the behaviour that was previously known as 'auto' (automatic migrations by hibernate). The new behavious of the 'auto' mode is to automatically apply the sql migration scripts provided within the 'database' docker image.
done |
@a-ba there is a conflict if you can resolve it ? |
Hello Anthony @a-ba ! The mysql server seems to start correctly, but the shanoir-entrypoint.sh does not seems to detect it:
In shanoir-entrypoint.sh l.40 this test
is never true, and after a test:
is the blocking part. When I removed it, it worked well. Maybe it is linked to my mysql version ? Some people actually try to connect to the database in such loop in entrypoint.shto wait for the disponibility, or check the logs for the "ready for connections." (but I find it quite ugly). Jean-Côme |
Hello @a-ba, |
@michaelkain there are conflicts I don't know how to resolve in entrypoint_common. Besides, I don't know exactly what to to with the new sql scripts. |
New SQL scripts should be renamed and moved to match the naming Anthony used: |
In https://springhow.com/spring-boot-database-initialization/#h-so-what-s-with-spring-jpa-generate-ddl says:
|
This commit:
database
imagemigrations
to track the list of applied migrationsdatabase
image to have the migrations applied on startup and controlled with the SHANOIR_MIGRATION environment variablePending issues:
SHANOIR_MIGRATION=auto
automatically attempts to apply the migration scripts, which may interfere with the other containers (because the microservices will also attempt to update their DB onSHANOIR_MIGRATION=auto
). Perhaps it would better to have the database container to do nothing onSHANOIR_MIGRATION=auto
)the ordering of migrations has to be deterministic and we may have an issue when dealing with long-lived branches. If any release happens before the branch is merged, then the ordering may be broken. We should have a CI test to ensure that all migrations added since the master branch have a greater sequence number.