Skip to content

Harden loader temp directory logic#404

Merged
filiph merged 4 commits intoalnitak:mainfrom
filiph:fix/loader-temp-directory
Feb 28, 2026
Merged

Harden loader temp directory logic#404
filiph merged 4 commits intoalnitak:mainfrom
filiph:fix/loader-temp-directory

Conversation

@filiph
Copy link
Copy Markdown
Collaborator

@filiph filiph commented Feb 25, 2026

Description

This change addresses a few issues with the current I/O loader:

  • It no longer tries to delete the temprorary directory in cleanUp. This would lead to the directory being deleted just after initialization when automaticCleanup is true, which is a catastrophic failure.
  • It creates the temporary directory with recursive: true, trying to avoid failures when the directory returned by path_provider doesn’t yet exist.
  • If the directory supplied by path_provider fails for any reason (I'm seeing this in the wild on macOS), use the system temp directory instead.
  • It adds stack trace to the severe log in initialization, and downgrades cleanup severe logs to warning logs.

Type of Change

  • ✨ New feature (non-breaking change which adds functionality)
  • 🛠️ Bug fix (non-breaking change which fixes an issue)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactor
  • ✅ Build configuration change
  • 📝 Documentation
  • 🗑️ Chore

This change addresses a few issues with the current I/O loader:

- It no longer tries to delete the temprorary directory in `cleanUp`. This would lead to the directory being deleted just after initialization when `automaticCleanup` is `true`.
- It creates the temporary directory with `recursive: true`, trying to avoid failures when the directory returned by `path_provider` doesn’t yet exist.
- It adds stack trace to the severe log in initialization, and downgrades cleanup severe logs to warning logs.
@filiph
Copy link
Copy Markdown
Collaborator Author

filiph commented Feb 25, 2026

No hurry to land this. I'll test it in the wild first, to see if it actually makes things better.

@alnitak
Copy link
Copy Markdown
Owner

alnitak commented Feb 25, 2026

That's great @filiph! This comes in time for the (already closed!) issue in this post.

Let me know when you feel like merging.

This also adds a fine-level log. Until now, the clean up was pretty much invisible in the wild because the only log call was `_log.finest()`, which is almost always ignored.
@filiph
Copy link
Copy Markdown
Collaborator Author

filiph commented Feb 28, 2026

I think this is ok — I haven't seen any reports from the wild.

@filiph filiph merged commit 0c43066 into alnitak:main Feb 28, 2026
1 check passed
@filiph filiph deleted the fix/loader-temp-directory branch February 28, 2026 19:49
@alnitak
Copy link
Copy Markdown
Owner

alnitak commented Feb 28, 2026

Great! Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants