feat(docker): add environment values to set log file for worker and beat#40998
Conversation
Code Review Agent Run #43fdecActionable Suggestions - 0Additional Suggestions - 1
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
Code Review Agent Run #1b8357Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
rusackas
left a comment
There was a problem hiding this comment.
Took this for a spin and I think the default actually does change behavior here, despite the "defaults unchanged" note. When WORKER_LOG_FILE/BEAT_LOG_FILE are unset the flag expands to --logfile=-, and Celery doesn't treat - as stdout... it just opens a file literally named - in the cwd (/app). Confirmed against the 5.4.0 we ship:
_detect_handler("-") -> WatchedFileHandler /app/-
_detect_handler(None) -> StreamHandler -> <stderr>
So a default docker-compose deploy would stop logging to docker logs and quietly write to a file called - instead. Could we only add the flag when the var is actually set? Something like:
${WORKER_LOG_FILE:+--logfile="$WORKER_LOG_FILE"}That keeps the current stderr behavior when unset, and drops the need for the HYPHEN_SYMBOL dance.
(fwiw I think the codeant bot's word-split warning is a false alarm here, the expansion is already quoted, and docker/.env is operator-controlled anyway.)
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Thanks @rusackas for the review. I tested your suggestion for setting and unsetting the destination, and it worked as expected. Updated the code accordingly. |
Code Review Agent Run #1ddb61Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
SUMMARY
This PR introduces two new environment variables,
WORKER_LOG_FILEandBEAT_LOG_FILE, to configure log file destinations for the worker and beat in the default Docker environment.Default values remain unchanged to ensure backward compatibility.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION