-
-
Notifications
You must be signed in to change notification settings - Fork 16.5k
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
Make CONFIG_DIR configurable per environment variable #4727
Make CONFIG_DIR configurable per environment variable #4727
Conversation
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.
👋 Hello @joaodiogocosta, thank you for submitting a 🚀 PR! To allow your work to be integrated as seamlessly as possible, we advise you to:
- ✅ Verify your PR is up-to-date with origin/master. If your PR is behind origin/master an automatic GitHub actions rebase may be attempted by including the /rebase command in a comment body, or by running the following code, replacing 'feature' with the name of your local branch:
git remote add upstream https://github.com/ultralytics/yolov5.git
git fetch upstream
git checkout feature # <----- replace 'feature' with local branch name
git rebase upstream/master
git push -u origin -f
- ✅ Verify all Continuous Integration (CI) checks are passing.
- ✅ Reduce changes to the absolute minimum required for your bug fix or feature addition. "It is not daily increase but daily decrease, hack away the unessential. The closer to the source, the less wastage there is." -Bruce Lee
e63ab42
to
0b68083
Compare
@joaodiogocosta looks good, thanks for the solution! I also started another PR before seeing this one in #4726. These two don't conflict so we may also merge the second one. PR is merged. Thank you for your contributions to YOLOv5 🚀 and Vision AI ⭐ |
Cool! Happy to help ✌️ |
WHY
CONFIG_DIR
, which is set togeneral.user_config_dir()
is currently hardcoded to be prefixed withPath.home()
.This change breaks all calls to YOLOV5 in environments that don't allow disk writes to
/home/{...}
, such as AWS Lambda and Google Cloud Functions.yolov5/utils/plots.py
Line 23 in 0d8a184
yolov5/utils/general.py
Lines 106 to 112 in 0d8a184
While the subdirectory takes into consideration the host OS defaults for configuration folders, there are systems in which
/home/{...}
is not writable.This can happen in any system really, depending on how filesystem access is configured. However, this issue becomes more prevalent for Docker containers, especially in environments like AWS Lambda and Google Cloud Functions.
These kind of environments don't allow disk writes to any directory other than
/tmp
. Functions are executed in a protected sandbox environment in which all disk writes must happen within/tmp
.Any attempt to write to directories other than
/tmp
, will throw an exception like the following:HOW
This Pull Request attempts to fix this hardcoding issue by introducing a configuration environment variable
YOLOV5_CONFIG_DIR
that can be set in the host environment or on a per execution basis.This approach is similar to what pytorch does with
TORCH_HOME
.Matplotlib is another example with their environment variable
MPLCONFIGDIR
.I'm happy to change the variable name as you see fit, or switch to a different approach we find a better one.
CLOSING NOTES
I hope this small patch makes life easier for those who are running YOLOV5 in such an environment.
🛠️ PR Summary
Made with ❤️ by Ultralytics Actions
🌟 Summary
Enhancement of configuration directory flexibility for YOLOv5.
📊 Key Changes
os
module to theplots.py
file.CONFIG_DIR
to prioritize theYOLOV5_CONFIG_DIR
environment variable if set, over the default user configuration directory.🎯 Purpose & Impact
YOLOV5_CONFIG_DIR
), providing greater flexibility and control in different environments or use cases.