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

Refactor docker-compose.yml for development #4544

Merged
merged 5 commits into from
May 11, 2020

Conversation

ariarijp
Copy link
Member

@ariarijp ariarijp commented Jan 14, 2020

What type of PR is this? (check all applicable)

  • Refactor

Description

Simplify docker-compose.yml by Extension fields.

Copy link
Member

@arikfr arikfr left a comment

Choose a reason for hiding this comment

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

Thanks! It's definitely better to keep it DRY. Just one comment about the version.

# For a production example please refer to getredash/setup repository on GitHub.
version: '3.4'
Copy link
Member

Choose a reason for hiding this comment

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

Is the version upgrade necessary? In the past we had issues with newer versions of compose configuration and usually there was little benefit in them.

Copy link
Member Author

@ariarijp ariarijp Jan 14, 2020

Choose a reason for hiding this comment

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

@arikfr Extention fields added in v3.4 format in my understanding.

I'll check it again.

In the past we had issues with newer versions of compose configuration and usually there was little benefit in them.

Could I have that issue URL?
I'd like to check that.

Copy link
Member Author

Choose a reason for hiding this comment

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

@arikfr I could not use Extention Fields on v3.2 format of docker-compose file.

However, we can use that if we can go back to v2 format like docker-compose.yml in setup repository.
Here is output of git diff between v3.4 style and v2 style.

diff --git a/docker-compose.yml b/docker-compose.yml
index bfeb38b7..904f89bb 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -1,12 +1,10 @@
 # This configuration file is for the **development** setup.
 # For a production example please refer to getredash/setup repository on GitHub.
-version: '3.4'
+version: "2"
 x-redash-service: &redash-service
   build: .
   volumes:
-    - type: bind
-      source: .
-      target: /app
+    - ".:/app"
 x-redash-environment: &redash-environment
   REDASH_REDIS_URL: "redis://redis:6379/0"
   REDASH_MAIL_DEFAULT_SENDER: "[email protected]"

IMHO, I think we can stay in the old version, but we should upgrade to 3.4 or higher for the future development if we could not get big benefit.

Copy link
Member

Choose a reason for hiding this comment

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

If we don't need any feature from 3.4, let's use 2.

Copy link
Member Author

Choose a reason for hiding this comment

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

@arikfr
Sure, I changed this PR can be run on v2 configulation.
Could you review it again?

Thanks, and stay healthy.

@arikfr arikfr merged commit e470347 into getredash:master May 11, 2020
@arikfr
Copy link
Member

arikfr commented May 11, 2020

Thanks!

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