fix: make LOG_DIR creation race-safe with exist_ok=True#9414
Open
pranav-afk wants to merge 1 commit into
Open
Conversation
Concurrent backend containers can hit FileExistsError when creating the logs directory during a clean local Docker Compose boot. Use exist_ok so parallel starts do not crash. Fixes makeplane#9352
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe local and production settings now create ChangesLog directory initialization
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Makes log directory creation race-safe during concurrent local Docker Compose startups.
On a clean boot, multiple backend containers (API, migrator, worker, beat) can all try to create plane/logs at the same time. The old pattern:
if not os.path.exists(LOG_DIR):
os.makedirs(LOG_DIR)
is not atomic, so one process can create the directory between another’s existence check and makedirs, causing:
FileExistsError: [Errno 17] File exists: '/code/plane/logs'
That can crash containers and leave the UI stuck loading—especially on multi-core hosts or Windows/WSL2 where the race window is wider.
Change: use a single race-safe call in both local and production settings:
os.makedirs(LOG_DIR, exist_ok=True)
Type of Change
• [x] Bug fix (non-breaking change which fixes an issue)
• [ ] Feature (non-breaking change which adds functionality)
• [ ] Improvement (change that would cause existing functionality to not work as expected)
• [ ] Code refactoring
• [ ] Performance improvements
• [ ] Documentation update
Screenshots and Media (if applicable)
N/A — backend settings only.
Test Scenarios
• Remove or clear the local apps/api/plane/logs directory (or start from a clean volume)
• Run docker compose -f docker-compose-local.yml up --build so api/migrator/worker start together
• Confirm containers start without FileExistsError in migrator/api logs
• Confirm the logs directory is created and services continue past settings import
• Re-run with an existing logs directory and confirm startup still succeeds (no-op path)
References
• Fixes #9352
Summary by CodeRabbit