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

Added development db for Sanctuary #78

Merged
merged 3 commits into from
Jan 30, 2024
Merged

Added development db for Sanctuary #78

merged 3 commits into from
Jan 30, 2024

Conversation

MadelaineJ
Copy link
Collaborator

Problem
We need to create a database for Sanctuary

Solution

  • added script to db container to create sanctuary db
  • modified alembic to have one central alembic folder that contains folders for each db we want migrations for
  • modified docker-compose.yaml and Makefile to run migrations for both databases
  • modified Intakes.py to be added to sanctuary db with BaseSanctuary
  • added UserMedical and UserSantuary class that inherit from user class.

TODOs:

  • Create separate user and pass for accessing sanctuary db
  • Write intstructions for adding a new database in README
  • Rename all instances of "example" db to "medical"
  • Tested that users and encounters can still be added to Medical db through api

Ticket URL
https://mediform.atlassian.net/browse/MEDI-30

Documentation
https://docs.google.com/document/d/10Pz324rr6zxEArmOlQne0Kwj9Ow61iKnUfkfU9OH8tY/edit

Tests Run
make clean && make all && make env -> creates the database make automig -> generates appropriate revisions
make mig -> applies appropriate revisions to appropriate database

Problem
We need to create a database for Sanctuary

Solution
- added script to db container to create sanctuary db
- modified alembic to have one central alembic folder that contains folders for each db we want migrations for
- modified docker-compose.yaml and Makefile to run migrations for both databases
- modified Intakes.py to be added to sanctuary db with BaseSanctuary
- added UserMedical and UserSantuary class that inherit from user class.

TODOs:
- Create separate user and pass for accessing sanctuary db
- Write intstructions for adding a new database in README
- Rename all instances of "example" db to "medical"
- Tested that users and encounters can still be added to Medical db through api

Ticket URL
https://mediform.atlassian.net/browse/MEDI-30

Documentation
https://docs.google.com/document/d/10Pz324rr6zxEArmOlQne0Kwj9Ow61iKnUfkfU9OH8tY/edit

Tests Run
make clean && make all && make env -> creates the database
make automig -> generates appropriate revisions
make mig -> applies appropriate revisions to appropriate database
@MadelaineJ MadelaineJ requested a review from critch646 January 30, 2024 00:46
@critch646
Copy link
Collaborator

Code quality is excellent!

I tested your branch locally and there is an issue. I followed your test instructions and I noticed some odd behaviour: If I do make all the migrations wouldn't happen. I would have to use make mig first the do make all for the db migration to be performed, and I figured out why.

In the Makefile, the all target executes docker compose up -d web which then docker compose the dependencies for web in the yaml file and starts services accordingly while the yaml for migs has - DB_NAME=${DB_NAME} and command: bash -c "cd /app/api && alembic --name=${DB_NAME} upgrade head" In the terminal I keep getting

WARN[0000] The "DB_NAME" variable is not set. Defaulting to a blank string.
WARN[0000] The "DB_NAME" variable is not set. Defaulting to a blank string.

Whenever I run anything but make mig. I added the DB_NAME to my .env and gave it some value, but the error keeps happening

Also if I run make all before make mig and I check the logs for migs I get this:

critch646@HYRDA-WIN:~/source/repos/project-ct$ docker compose logs migs
WARN[0000] The "DB_NAME" variable is not set. Defaulting to a blank string.
WARN[0000] The "DB_NAME" variable is not set. Defaulting to a blank string.
migs  | FAILED: No config file 'alembic.ini' found, or file has no '[]' section

@MadelaineJ MadelaineJ closed this Jan 30, 2024
@MadelaineJ MadelaineJ reopened this Jan 30, 2024
@MadelaineJ
Copy link
Collaborator Author

It should be fixed. Issue was that some services rely on migs. Those services weren't passing DB_NAME.

I changed it to make another service for migssanctuary. I didn't make the other services rely on it. I think we can worry about including sanctuary migrations for production environment later.

@critch646
Copy link
Collaborator

I pulled your changes and when I do make all it works for medical db but not sanctuary. In the docker-compose.yaml file for the api service if you add - migssanctuary under depends_on: it will start the that service as well.

@MadelaineJ
Copy link
Collaborator Author

Done

Copy link
Collaborator

@critch646 critch646 left a comment

Choose a reason for hiding this comment

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

you did it!

@MadelaineJ MadelaineJ merged commit 50b0d9b into main Jan 30, 2024
@MadelaineJ MadelaineJ deleted the madelaine branch January 30, 2024 17:41
critch646 added a commit that referenced this pull request Jan 30, 2024
critch646 added a commit that referenced this pull request Jan 30, 2024
This reverts commit 50b0d9b.

Problem
The new migrations are not working in production as expected.
critch646 added a commit that referenced this pull request Jan 30, 2024
* Revert "added --name=medical to dockerfile.prod"

This reverts commit d4828b9.

* Revert "fixing github actions failure (again) (#80)"

This reverts commit ba5a515.

* Revert "Resolving Github actions failure (#79)"

This reverts commit 82e27da.

* Revert "Added development db for Sanctuary (#78)"

This reverts commit 50b0d9b.

Problem
The new migrations are not working in production as expected.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants