GMOD / Chado

the GMOD database schema
http://gmod.org/wiki/Chado
Artistic License 2.0
38 stars 25 forks source link

Automated testing for applying migrations in a named schema #146

Closed laceysanderson closed 4 months ago

laceysanderson commented 4 months ago

Issue #145

This PR builds on and is dependant on PR #143. It alters the dockerfile to allow you to name the schema you want chado installed in and creates some automated workflows that do this. This way we can confirm that there is nothing in the migration that is specific to the public schema!

Testing

This really just needs a code review and to confirm the automated testing passes. If you want to be extra thorough or are curious, then you can build the image with a set schema name as follows:

docker build --tag gmod/chado:local \
  --file docker/Dockerfile \
  --build-arg SCHEMA_NAME="teacup" ./
docker run -it --rm --volume=$(pwd):/Chado gmod/chado:local

This will install chado into a schema named teacup. You can use psql -U postgres -h localhost -d chado with the password chadotest to look at the schema. Specifically, \dt public.* should only have a flyway table and \dt teacup.* should list out the whole chado schema + a flyway migrations table.

spficklin commented 4 months ago

Code changes to the Dockerfile look good and makes sense to me. I ran the test and the docker built just fine and the tests performed as described. I also applied the migrations and that worked as well.

One question though I'm getting this message when I fire up the Docker image:

37 SQL migrations were detected but not run because they did not follow the filename convention.

Do you know the cause of that? Whatever is causing that doesn't seem to interfere with the migration but it may create confusion when folks see it.

spficklin commented 4 months ago

I'm approving as it works as expected. My question above is just a question.

spficklin commented 4 months ago

An update. @laceysanderson and I spoke by Slack and she reminded me that the warning about the 37 SQL migrations that don't follow the migration naming scheme are left over from when we migrated Chado to use Flyway. This can probably be fixed if we dig into it. This warning has been present for quite a while and not related to this PR.

laceysanderson commented 4 months ago

Great, thanks @spficklin! Are you ok with me merging this one as well @scottcain? It simply extends the last one to allow Chado to be installed into a separate schema in the docker and creates a workflow that tests this with schema installed in a schema named teacup 🍵 🤪 . Not Tripal specific in any way :-)