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

feat: Modify docker-compose yaml to read region and default region from env… #446

Merged
merged 4 commits into from
May 10, 2023

Conversation

dlpzx
Copy link
Contributor

@dlpzx dlpzx commented May 9, 2023

Feature or Bugfix

  • Bugfix
  • Refactoring

Detail

Added AWS_REGION to the environment variables of the Docker containers for local development. Set bothAWS_DEFAULT_REGION and AWS_REGION to their values set on the terminal where docker-compose up is run.
If these values are not set, eu-west-1 is used as default
Another PR with better instructions to the github pages documentation (deploy locally) will follow.

Relates

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@@ -16,7 +16,8 @@ services:
- db
environment:
envname: 'dkrcompose'
AWS_DEFAULT_REGION: "eu-west-1"
AWS_REGION: "${AWS_REGION}"
Copy link
Contributor

Choose a reason for hiding this comment

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

We could consider specify the default values if the env variables are not specify. e.g.:

AWS_REGION: "${AWS_REGION:-eu-west-1}"
AWS_DEFAULT_REGION: "${AWS_DEFAULT_REGION:-eu-west-1}"

Otherwise, we have to provide them all the time from the console

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, as it is it already defaults to Ireland in the backend code, but I think it is better to set the region default earlier, not in the last piece of the workflow. Added in the last commit

@dlpzx dlpzx requested a review from nikpodsh May 10, 2023 06:03
@dlpzx
Copy link
Contributor Author

dlpzx commented May 10, 2023

@nikpodsh yesterday GitHub faced some issues with their APIs and I had some issues commiting your suggestion. But it is there now

@dlpzx dlpzx merged commit 13a2fc0 into data-dot-all:main May 10, 2023
@dlpzx dlpzx deleted the fix-docker-region branch May 16, 2023 06:39
@dlpzx dlpzx changed the title Modify docker-compose yaml to read region and default region from env… feat: Modify docker-compose yaml to read region and default region from env… May 24, 2023
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.

4 participants