-
Notifications
You must be signed in to change notification settings - Fork 42.8k
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
Prevent docker compose to break config by creating folders #4125
Prevent docker compose to break config by creating folders #4125
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #4125 +/- ##
==========================================
+ Coverage 64.53% 64.76% +0.22%
==========================================
Files 75 75
Lines 3542 3542
Branches 520 520
==========================================
+ Hits 2286 2294 +8
+ Misses 1086 1078 -8
Partials 170 170
☔ View full report in Codecov by Sentry. |
fcd3002
to
82792b7
Compare
Maybe just my 2 cents.. what if we specifically check for those files on start and tell the user specifically that those are missing? |
The AI config is ok to be missing in the general case, it will be created. For azure - that must be there if you want to use azure. The issue is that I cannot test azure and afaik it has been broken in the last couple of releases. Feel free to make a PR if you want. |
@Wladastic happy to approve and get this in v0.3.2? I'll float it by azure testers |
This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request. |
Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly. |
I got azure access but it costs a fortune to keep up all the time. Let me know if you need keys or me to test. Azure works fine right now as far as I can tell. The docs are just bad |
Then I think we are good to merge. There is nothing here to test with the actual azure. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't tested it but it looks good
Background
Docker compose will create folders instead of file for volumes if they do not exist on the host. That would cause error on start:
Changes
Switch the Volume type in docker-compose.yml documentation to use bind type. That would require users to have the files already created.
Added default values to be empty dictionary in case uses have created the files but left them empty preventing this error:
Linked issues:
#3052
PR Quality Checklist