fix(viewer): use writable cache dir for thumbnail generation#167
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds configurable thumbnail cache directory selection (env-var, media-root subdirectory, or temp fallback), updates the thumbnail API to accept an optional cache directory parameter with validation, integrates lazy cache initialization in the web endpoint, and bumps the version to 7.10.11. ChangesVersion 7.10.11 Release with Configurable Thumbnail Cache
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
🐳 Dev images published!
The dev/test instance will pick up these changes automatically (Portainer GitOps). To test locally: docker pull drumsergio/telegram-archive:dev
docker pull drumsergio/telegram-archive-viewer:dev |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/web/thumbnails.py`:
- Around line 40-52: The function currently returns a directory after mkdir(...)
which can succeed on an existing read-only dir and will raise if creating
THUMBNAIL_CACHE_DIR fails; update the code that handles env_dir and media_root
to catch exceptions from Path(env_dir).mkdir and, on success, verify the
directory is actually writable before returning (e.g., attempt to create and
remove a small temp file or use os.access(candidate, os.W_OK) with a real write
test); do the same for candidate (media_root / ".thumbs") inside the try/except
so that if creation or writability check fails the function falls back instead
of returning an unwritable path. Ensure references to env_dir, candidate,
media_root, Path, and mkdir are used to locate the changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 39c04e46-664e-454e-b46c-d67d8ebb178e
📒 Files selected for processing (4)
pyproject.tomlsrc/__init__.pysrc/web/main.pysrc/web/thumbnails.py
| env_dir = os.environ.get("THUMBNAIL_CACHE_DIR") | ||
| if env_dir: | ||
| p = Path(env_dir) | ||
| p.mkdir(parents=True, exist_ok=True) | ||
| return p | ||
|
|
||
| if media_root: | ||
| candidate = media_root / ".thumbs" | ||
| try: | ||
| candidate.mkdir(parents=True, exist_ok=True) | ||
| return candidate | ||
| except OSError: | ||
| pass |
There was a problem hiding this comment.
Validate actual writability before returning a cache directory.
mkdir(..., exist_ok=True) can succeed for an existing read-only directory, so this may still return an unwritable {media_root}/.thumbs and reproduce the thumbnail failure. Also, if THUMBNAIL_CACHE_DIR creation fails, the function currently raises instead of falling back.
Suggested fix
+def _is_writable_dir(path: Path) -> bool:
+ try:
+ path.mkdir(parents=True, exist_ok=True)
+ probe = path / ".write_probe"
+ with probe.open("wb"):
+ pass
+ if probe.exists():
+ probe.unlink()
+ return True
+ except OSError:
+ return False
+
def resolve_cache_dir(media_root: Path | None) -> Path:
@@
env_dir = os.environ.get("THUMBNAIL_CACHE_DIR")
if env_dir:
p = Path(env_dir)
- p.mkdir(parents=True, exist_ok=True)
- return p
+ if _is_writable_dir(p):
+ return p
- if media_root:
+ if media_root is not None:
candidate = media_root / ".thumbs"
- try:
- candidate.mkdir(parents=True, exist_ok=True)
+ if _is_writable_dir(candidate):
return candidate
- except OSError:
- pass
p = Path(_DEFAULT_CACHE_DIR)
p.mkdir(parents=True, exist_ok=True)
return p🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/web/thumbnails.py` around lines 40 - 52, The function currently returns a
directory after mkdir(...) which can succeed on an existing read-only dir and
will raise if creating THUMBNAIL_CACHE_DIR fails; update the code that handles
env_dir and media_root to catch exceptions from Path(env_dir).mkdir and, on
success, verify the directory is actually writable before returning (e.g.,
attempt to create and remove a small temp file or use os.access(candidate,
os.W_OK) with a real write test); do the same for candidate (media_root /
".thumbs") inside the try/except so that if creation or writability check fails
the function falls back instead of returning an unwritable path. Ensure
references to env_dir, candidate, media_root, Path, and mkdir are used to locate
the changes.
The viewer container mounts the media volume read-only, causing thumbnail generation to fail with EROFS. Thumbnails are now cached in a separate writable directory (THUMBNAIL_CACHE_DIR env, or /tmp/telegram-archive-thumbs as fallback).
76ae975 to
f70bff3
Compare
|
🐳 Dev images published!
The dev/test instance will pick up these changes automatically (Portainer GitOps). To test locally: docker pull drumsergio/telegram-archive:dev
docker pull drumsergio/telegram-archive-viewer:dev |
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (61.76%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #167 +/- ##
==========================================
- Coverage 92.93% 92.77% -0.16%
==========================================
Files 22 22
Lines 6466 6492 +26
==========================================
+ Hits 6009 6023 +14
- Misses 457 469 +12
🚀 New features to boost your workflow:
|
Summary
[Errno 30] Read-only file systembecause the viewer container mounts the media volume as read-only (ro){media_root}/.thumbs/THUMBNAIL_CACHE_DIRenv →{media_root}/.thumbs(if writable) →/tmp/telegram-archive-thumbsTest plan
Summary by CodeRabbit
New Features
Chores