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

Modify db docker service to use the env file #437

Merged
merged 5 commits into from
Feb 4, 2025

Conversation

del9ra
Copy link
Member

@del9ra del9ra commented Nov 14, 2024

Fixes #309

What changes did you make?

  • moved the environment variables from docker-compose.yml file to .env.docker-example
  • replaced the environment section with env_file: - ./app/.env.docker-example in docker-compose.yml
  • change the django settings (settings.py) code to use the database variable names that the db container uses

Why did you make the changes (we will use this info to test)?

  • to update the db Docker container to use the same .env file as the web container, so we avoid hard-coding values in the docker-compose.yml file

@del9ra del9ra force-pushed the modify-db-docker-service-309 branch from 2cc0bd9 to 5e12156 Compare November 14, 2024 01:57
Copy link
Member

@fyliu fyliu left a comment

Choose a reason for hiding this comment

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

Thanks for working on this!

I slack messaged Ethan about the sqlite. I think we need to wait for his response to determine how to name the env variables. But, ultimately, we want to get rid of the docker-compose.yml variables like you did here.

@del9ra
Copy link
Member Author

del9ra commented Nov 15, 2024

  1. Almost done
  2. Blocker: I changed each SQL affix to POSTGRES under this section: # postgres settings for local development, but I didn’t change anything under # sqlite settings for local development since it’s for SQLite. I only changed the variable names, not the values.
    Do I need to replace the SQL prefix with POSTGRES in the .env.docker file as well?
  3. ETA - November 15

@del9ra del9ra requested a review from fyliu November 17, 2024 01:44
Copy link
Member

@fyliu fyliu left a comment

Choose a reason for hiding this comment

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

Sorry I wasn't more clear before. I was also not thinking clearly about the scope of this issue. We're just trying to remove the hardcoded lines in the compose file. Since the project had support for sqlite, we should just let it keep doing that and add a workaround to do both. I wrote the details on what to do in the comments.

@del9ra del9ra requested a review from fyliu November 19, 2024 20:59
fyliu
fyliu previously requested changes Nov 20, 2024
Copy link
Member

@fyliu fyliu left a comment

Choose a reason for hiding this comment

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

I think this is not too obvious to test, but if you copied app/.env.docker-example to app/.env.docker and ran ./scripts/buildrun.sh, it actually doesn't run. I put the reason for that in the comments. Thanks for working through this!

@del9ra del9ra force-pushed the modify-db-docker-service-309 branch from 294192f to 3159fdc Compare November 20, 2024 23:08
@del9ra del9ra requested a review from fyliu November 20, 2024 23:10
@del9ra del9ra mentioned this pull request Jan 31, 2025
9 tasks
@fyliu fyliu force-pushed the modify-db-docker-service-309 branch from 3159fdc to b0ffb25 Compare February 4, 2025 00:00
@fyliu fyliu dismissed their stale review February 4, 2025 00:09

I'm not sure what this refers to, but the code changes look good and is working right now.

Copy link
Member

@fyliu fyliu left a comment

Choose a reason for hiding this comment

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

Thanks for contributing these changes! The code looks good and it's running well.

@fyliu fyliu merged commit 20af4e8 into hackforla:main Feb 4, 2025
1 check passed
fyliu pushed a commit that referenced this pull request Feb 4, 2025
* modifed db docker service in docker-compose.yml to use the env file
* updated postgres settings for local development in env.docker-example
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅Done
Development

Successfully merging this pull request may close these issues.

Modify db docker service to use the env file
2 participants