fix: fallback behaviour added to atomic writes#5748
fix: fallback behaviour added to atomic writes#5748suleman1412 wants to merge 13 commits intoprefix-dev:mainfrom
Conversation
baszalmstra
left a comment
There was a problem hiding this comment.
You have to check whether this actually works. Not just of the temp file is in the right directory. The assumption is that if the directory is not writable you still can overwrite the file. We need to check this.
|
@baszalmstra persist fails in a readonly directory. In such cases we can fallback to using fs:write() instead, which wouldnt be non atomic but should work. |
Note the previous behavior was fine (I'd imagine the fallback should be to the original behavior that existed in |
|
Yep, that sounds good to me! |
|
hi @baszalmstra, if directory is readonly then we fallback to use write along with a warning about the loss of atomicity. Addded e2e tests for the same as well, please have a look. |
| /// the same directory and then renaming it to the target path. | ||
| tempfile::Builder::new() | ||
| .prefix(&prefix) | ||
| .tempfile_in(target_dir) |
There was a problem hiding this comment.
Cant we just return an error from this function and catch the "permission denied error".
Description
Implemented a fallback behaivour for when the parent directory is not writeable, then falls back to $TMPDIR
Added tests to check :
Fixes atomic writes in pixi assume the
pixi.tomlpath is writeable #5724How Has This Been Tested?
AI Disclosure
Tools: Claude
Checklist:
schema/model.py.