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

chore: add .env.example and update docker-compose.yaml #5339

Closed
wants to merge 6 commits into from

Conversation

Tian-Hun
Copy link
Contributor

Description

  • Added .env.example file to provide a template for environment variable configuration.
  • Modified docker-compose.yaml to streamline configuration management by using environment variables.

Fixes #5249

Type of Change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update, included: Dify Document
  • Improvement, including but not limited to code refactoring, performance optimization, and UI/UX improvement
  • Dependency upgrade

How Has This Been Tested?

To verify the changes introduced in this pull request, the following tests were performed:

  1. Environment Variable Validation:

    • Description: The updated docker-compose.yaml now references environment variables from the new .env.example file. These environment variables were set with representative values in a local .env file to ensure that the Docker Compose services start up correctly and can access the necessary configuration.
    • Reproduction:
      1. Create a .env file in the same directory as docker-compose.yaml.
      2. Populate the .env file with the values for your environment variables (following the structure provided in .env.example).
      3. Run docker-compose up to start the services.
      4. Verify that the services start without errors and that the environment variables are correctly loaded by checking the logs or accessing the application.
  2. Configuration File Validation:

    • Description: The modified docker-compose.yaml file was manually reviewed to ensure that all environment variable references are correct and that the overall configuration structure aligns with the intended changes.
    • Reproduction:
      1. Examine the docker-compose.yaml file and verify that all environment variable placeholders (e.g., ${VARIABLE}) are replaced with the correct values from your .env file.
      2. Check that the structure of the configuration file (service definitions, volume mounts, etc.) is valid and makes sense for your setup.

Test Configuration:

  • Environment: Local development environment
  • Docker Compose Version: v2.15.1
  • Operating System: macOS Sonoma 14.5

Additional Notes:

  • Depending on the complexity of your application and its dependencies, you might need to conduct more thorough testing. This could include functional tests, integration tests, or performance tests.
  • If the changes involve external services (e.g., cloud storage, databases), ensure those services are available and configured correctly during testing.

Suggested Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I ran dev/reformat(backend) and cd web && npx lint-staged(frontend) to appease the lint gods
  • optional I have made corresponding changes to the documentation
  • optional I have added tests that prove my fix is effective or that my feature works
  • optional New and existing unit tests pass locally with my changes

- Added .env.example file to provide a template for environment variable configuration.
- Modified docker-compose.yaml to streamline configuration management by using environment variables.

Fixes langgenius#5249
@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. 💪 enhancement New feature or request 📚 documentation Improvements or additions to documentation labels Jun 18, 2024
Copy link
Member

@crazywoola crazywoola left a comment

Choose a reason for hiding this comment

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

This is a good pr, but you need to move the comments to .env.examaple as well.

@crazywoola
Copy link
Member

crazywoola commented Jun 18, 2024

See this one as well #4299 If you would like to discuss about this pr, you can add my wechat: crazyphage.

crazywoola
crazywoola previously approved these changes Jun 18, 2024
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jun 18, 2024
@Tian-Hun
Copy link
Contributor Author

Hi @takatost, @guchenhe, and @crazywoola , 👋

I'd really appreciate it if you could take a few minutes to look over this PR. Your feedback is invaluable to me! 🙏

@perzeuss
Copy link
Contributor

perzeuss commented Jun 23, 2024

Hi @Tian-Hun, great to see a new contributor! I have some feedback I want to share.

It is a good idea to de-duplicate entries, but I think moving some variables to a .env file while others have to be configured in the docker-compose.yaml introduces problems. It creates two different ways to configure Dify.

It will make it more complicated to help users set up Dify if we need to check whether a variable is in a .env file or defined in docker-compose.yaml. It will confuse people who read GitHub issues/discussions about how to configure Dify if they follow instructions to configure things in the docker-compose.yaml file and then find that the required environment variables for a service are no longer in the service section but at the top of the file, and then realize there also needs to be a .env file for it.

I think if we refactor it this way, we should move all variables to .env files to only have the .env files as place to configure the services. I discussed this with @guchenhe a month ago, and here is an example I made to show how it could be done: perzeuss@893c9e8#diff-89fafc265c1fa601cf2364b20be83308031335fda9467fcf3249d5ec1c0c8172

Let me know what you think about my thoughts. By the way, thanks for taking the time to contribute!

@Tian-Hun
Copy link
Contributor Author

Tian-Hun commented Jun 24, 2024

Hi @perzeuss,

Thank you for your valuable feedback! I understand your concern about introducing two different ways to configure Dify.

The initial intention of splitting environment variables was to streamline the setup process for new users. By separating frequently changed and personalized variables into a .env file, we aimed to simplify initial deployment and focus their attention on essential configurations.

Additionally, by keeping the .env file out of version control and providing an .env.example template, we enable users to customize their local environments without worrying about conflicts when pulling updates from the repository. This streamlined workflow is crucial for a smooth user experience.

While centralizing all variables in .env files offers consistency, it's important to weigh this benefit against the potential for merge conflicts and the added complexity it might introduce for users who need to manage custom configurations.

I propose we explore a hybrid approach that combines the best of both worlds. We could keep essential, non-sensitive variables in the docker-compose.yaml file for easy reference and move more personalized or sensitive variables to .env files. This would maintain the convenience of a single configuration location for common settings while leveraging the flexibility and conflict resolution benefits of .env files for user-specific customizations.

I'm confident that by collaborating and considering all perspectives, we can find an optimal solution that prioritizes both user-friendliness and maintainability.

Thank you again for your insights and for taking the time to contribute to this discussion!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📚 documentation Improvements or additions to documentation 💪 enhancement New feature or request lgtm This PR has been approved by a maintainer size:XL This PR changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement] Optimization of the docker-compose.yaml file
3 participants