Repository navigation
Handle stale filesystem metadata when creating output directories - #108
Merged
Merged
Conversation
Replaced the 'if not os.path.isdir(x): os.makedirs(x)' pattern with a new ensureDir() helper in fsqcUtils. On networked filesystems (SMB/CIFS, NFS) os.path.isdir() may transiently return False for a directory that does exist. This makes os.makedirs() raise FileExistsError even with exist_ok=True, because it re-raises whenever its own isdir() check fails. The same outdated view also makes the to_csv() calls fail with pandas' "Cannot save file into a non-existent directory", which aborted long batch runs at the metrics.csv and status.txt writes. ensureDir() retries a few times with a short delay, and still raises once the attempts are exhausted; a non-directory file blocking the path is never removed.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
ensureDir() should validate retries/delay inputs (e.g., negative retries currently results in silently doing nothing), to avoid surprising behavior in a new shared utility.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR hardens fsqc’s output-directory creation and subsequent writes against filesystems that can temporarily present stale directory metadata (e.g., SMB/NFS), preventing intermittent FileExistsError / “non-existent directory” write failures as described in #107.
Changes:
- Added
ensureDir()utility that retriesos.makedirs(..., exist_ok=True)onFileExistsErrorto tolerate transient stale directory visibility. - Replaced multiple
if not isdir(): makedirs()/mkdir()patterns withensureDir()across the main workflow and motion-check module. - Added pytest regression tests covering directory creation, existing directories, simulated transient
FileExistsError, and persistent path-blocking files.
File summaries
| File | Description |
|---|---|
| fsqc/utils/tests/test_ensure_dir.py | Adds regression tests for tolerant directory creation under transient metadata staleness. |
| fsqc/fsqcUtils.py | Introduces ensureDir() helper with retry-on-FileExistsError behavior. |
| fsqc/fsqcMain.py | Routes output directory creation through ensureDir() and re-checks dirs immediately before to_csv() writes. |
| fsqc/checkMotion.py | Uses ensureDir() for mask output directory creation to avoid stale-metadata failures. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Summary
This pull request makes output directory handling tolerant of filesystems that report directory metadata with a delay, where
os.path.isdir()can transiently returnFalsefor a directory that does exist.With the current
if not os.path.isdir(x): os.makedirs(x)pattern, this can result inFileExistsError. The same stale directory view can later cause theto_csv()calls to fail with pandas'Cannot save file into a non-existent directory, terminating the entire run.This addresses and closes #107.
Changes
ensureDir()tofsqcUtils.py, which retriesos.makedirs(path, exist_ok=True)a few times before giving up.exist_ok=Truealone is not sufficient here because CPython can re-raiseFileExistsErrorwhen its ownisdir()check also sees a staleFalse.fsqcMain.pyand the additional site incheckMotion.py.ensureDir()immediately before the twoto_csv()writes where the affected runs were observed to fail.fsqc/utils/tests/test_ensure_dir.pywith regression tests for this behavior.Only
FileExistsErroris retried — other errors (e.g.PermissionError) propagate immediately — and a genuineFileExistsError(a non-directory file blocking the path) is still raised once the retries are exhausted, without removing that file.Notes
ensureDir()creates missing parent directories, whereas eight of the replaced sites previously usedos.mkdir(). In practice, this means that an output path such asa/b/cnow also works whena/bdoes not exist yet.The new test file was added with
git add -fbecause the repository's.gitignorecontains a baretestspattern that matchesfsqc/utils/tests/at any depth. The existing tests in that directory are tracked for the same reason. If preferred, I can also remove the new test file and keep the change limited to the implementation.