Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #35708 +/- ##
==========================================
- Coverage 85.73% 85.68% -0.05%
==========================================
Files 4461 4459 -2
Lines 209055 209095 +40
Branches 39075 39106 +31
==========================================
- Hits 179224 179166 -58
- Misses 26667 26766 +99
+ Partials 3164 3163 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I would suggest:
This approach keeps all the information related to configuration (E.G. comment, default value, generation rules for default value) in one piece, and avoid manually synchronizing comments and default value between configuration definitions and default configuration file. |
|
I agree with @QuantumGhost . Another PR will be added for those changes. |
|
@QuantumGhost Shall we centralize the configuration definitions from |
|
@Stream29 I would suggest centralizing ALL information related to configuration to code. We may need to introduce special annotations for Pydantic fields such as Currently, we have three pieces of default configuration:
Both the first one and the second one could be generated from Pydantic model configurations, though generation of the docker compose default configuration would need extra steps since it also includes configuration for other services, such as Postgres, Plugin Daemon and Dify Web. If we could complete the migration above, then the .env.example is no longer needed and should be removed. One caveat: the .env.example files in api and docker directories already diverge. Some configuration only exist in one of them and a comparison is need to merge them properly. |
|
@QuantumGhost
Some default values have been already moved to Pydantic configuration fields. But there are still some default values meaningful for first-time deployment. For example, database and base url configurations. Keeping these in
it's difficult to initialize the default configuration file from configuration model definitions with a Python script. |
|
I think this PR fixes #34321. |
|
Most of what the wrapper does can be replaced with native Compose features. The PR's 660 lines of bash/PS reimplement plumbing that Compose 2.20+ already provides. Ship defaults, user overrides User just runs `docker compose up -d. .env.default is committed, .env is gitignored. |
|
@crazywoola Good point. However, the |
|
The variable substitution logic in this PR introduces maintenance burden. I would propose replace env merge logic with |
This is will result in incorrect bahavior. For example: User may cover this variable in And this variable in |
Document: "set DB_TYPE=mysql and DB_HOST=db_mysql together." That's one extra line in the README vs. 100 lines of awk in the script. The wrapper's metadata_db_host/metadata_db_port/metadata_db_user switch logic is essentially duplicating profile semantics in shell. |
|
After some thought, I agree that sticking with Docker Compose is the better solution. For initial configuration generation (e.g., #34321), we could introduce a bootstrap script. This reduces the maintenance burden and provides a better user experience, as there are far more resources available for Docker Compose than for a home-grown script. Sticking with Docker Compose also gives Dify deployers more flexibility, since they can leverage any mechanism Compose supports (e.g., compose file merging), rather than being forced to read and modify the dify-compose script. |
|
@QuantumGhost I agree with you. A new PR will be added to fix this experience. |
Important
Fixes #<issue number>.Summary
Screenshots
Checklist
make lint && make type-check(backend) andcd web && pnpm exec vp staged(frontend) to appease the lint gods