fix: added conversation lock file management in LogManager #295
Conversation
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to 48b62ca in 26 seconds
More details
- Looked at
59lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_VdJV7VsGAc34lwoS
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| @@ -97,7 +117,15 @@ def __init__( | |||
| if _branch not in self._branches: | |||
| self._branches[_branch] = Log.read_jsonl(file) | |||
|
|
|||
| # TODO: Check if logfile has contents, then maybe load, or should it overwrite? | |||
| def __del__(self): | |||
There was a problem hiding this comment.
Using __del__ for releasing resources is not reliable. Consider using a context manager to ensure the lock is released properly.
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## master #295 +/- ##
==========================================
+ Coverage 72.24% 72.25% +0.01%
==========================================
Files 64 64
Lines 4331 4351 +20
==========================================
+ Hits 3129 3144 +15
- Misses 1202 1207 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Important
Adds optional directory locking to
LogManagerusingfcntlto prevent concurrent access.LogManagerusingfcntlin__init__..lockfile in log directory and acquires exclusive lock.RuntimeErrorif lock cannot be acquired.__del__to release lock and close file descriptor.This description was created by
for 48b62ca. It will automatically update as commits are pushed.