Enhacement for import existing media#485
Enhacement for import existing media#485juandbc wants to merge 8 commits intomaxdorninger:masterfrom
Conversation
Refactor utility methods into a dedicated module.
Refactor more util methods for files import
📝 WalkthroughWalkthroughThis pull request reorganises media file handling utilities by migrating functions from torrent utilities to a new centralised file handler module, whilst enhancing quality detection patterns, refactoring subtitle import logic across movie and TV services, and consolidating imports to the new file handler interface. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
juandbc
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
media_manager/torrent/service.py (1)
59-67:⚠️ Potential issue | 🟠 MajorKeep the repository state in sync after cancellation.
This change removes more than the return value:
cancel_download()no longer refreshes or persists the torrent status afterremove_torrent(...). Inmedia_manager/tv/service.py:565-575andmedia_manager/movies/service.py:405-420, the call is followed byraise, notdelete_torrent(), so those paths can leave a stale torrent row behind even though the download has already been removed. Please either persist a cancelled/removed status here or ensure those paths always delete the record immediately.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@media_manager/torrent/service.py` around lines 59 - 67, cancel_download currently removes the torrent from the download manager but does not persist any change to the torrent record, leaving a stale DB row; after calling self.download_manager.remove_torrent(torrent, delete_data=delete_files) either set torrent.status = "cancelled" (or "removed") and persist that change via the existing persistence API (e.g., self.torrent_repository.update / save) or, if callers expect immediate deletion, call the existing delete_torrent(torrent) helper here; update cancel_download to persist the status change or delete the record so callers in other services (which raise after calling cancel_download) cannot leave stale rows.media_manager/tv/service.py (1)
1017-1027:⚠️ Potential issue | 🔴 Critical
file_path_suffixmismatch: EpisodeFile record won't match actual filename.In
import_season(lines 664-666), the suffix is computed as:new_path_suffix = f"{QualityStrings.get_label(episode.quality)} - {file_path_suffix}"So when called with
file_path_suffix="IMPORTED", actual files are named with suffix like"1080p - IMPORTED". However, theEpisodeFilerecord created here usesfile_path_suffix="IMPORTED"(line 1023), which doesn't match the actual filename.This inconsistency could cause file lookups to fail when the application tries to locate episode files by their recorded suffix.
Compare with
movies/service.py(lines 569-581) which correctly uses the same computed suffix for both the import and the database record.🐛 Proposed fix
Either return the computed suffix from
import_seasonor compute it before calling:+ file_quality = extract_quality_video_file(video_files[0]) if video_files else Quality.unknown + computed_suffix = f"{QualityStrings.get_label(file_quality)} - IMPORTED" _success, imported_episodes = self.import_season( show=tv_show, season=season, video_files=video_files, subtitle_files=subtitle_files, - file_path_suffix="IMPORTED", + file_path_suffix=computed_suffix, ) for episode in imported_episodes: episode_file = EpisodeFile( episode_id=episode.id, quality=episode.quality, - file_path_suffix="IMPORTED", + file_path_suffix=computed_suffix, torrent_id=None, )Note: This also requires adjusting
import_seasonto not prepend the quality label again, or passing the raw suffix and lettingimport_seasonreturn the computed one.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@media_manager/tv/service.py` around lines 1017 - 1027, The EpisodeFile record is created with file_path_suffix="IMPORTED" but import_season builds filenames as f"{QualityStrings.get_label(episode.quality)} - {file_path_suffix}", causing mismatch; fix by ensuring the same computed suffix is used when persisting: either have import_season return the computed new_path_suffix and use that value when constructing EpisodeFile, or compute new_path_suffix locally before calling import_season (using QualityStrings.get_label(episode.quality) + " - " + file_path_suffix) and pass that exact string into EpisodeFile (used by tv_repository.add_episode_file) so stored file_path_suffix matches the actual filename.
🧹 Nitpick comments (1)
media_manager/tv/service.py (1)
678-700: Asymmetric exception handling:FileNotFoundErroris re-raised, other exceptions are swallowed.
FileNotFoundErrorat line 688 is re-raised after logging, while genericExceptionat line 689 setssuccess = Falseand continues processing. This asymmetry means:
- A missing file aborts the entire import
- Other errors (e.g., permission issues, disk full) allow the loop to continue
If this is intentional (fail-fast for missing files, best-effort for other errors), consider adding a brief comment to clarify the design rationale.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@media_manager/tv/service.py` around lines 678 - 700, The except handlers for FileNotFoundError and generic Exception in the import block are asymmetric: FileNotFoundError is logged and re-raised while other exceptions are logged, notify via notification_service.send_notification_to_all_providers, and only set success = False, which causes different control flow for missing files vs other errors. Decide and implement consistent behavior: either stop the import for all errors (remove success = False and re-raise in the generic Exception handler) or treat missing files as non-fatal (catch FileNotFoundError, log/notify and set success = False instead of re-raising); alternatively, if the current asymmetry is intentional, add a brief clarifying comment above the FileNotFoundError handler describing why missing files must fail-fast. Reference the FileNotFoundError except block, the generic except Exception block, log.exception calls, the success flag, and notification_service.send_notification_to_all_providers when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@media_manager/movies/service.py`:
- Around line 568-576: The code calls extract_quality_video_file(video_files[0])
without checking that video_files is non-empty, which can raise IndexError;
update the surrounding logic in the method to validate video_files (from
get_files_for_import) before using video_files[0], e.g., check len(video_files)
> 0 (or handle len(video_files) != 1 if that is the intended invariant), and if
empty log or raise a clear exception or return early instead of proceeding to
call extract_quality_video_file and import_movie; make the check near where
file_quality is computed and ensure import_movie is only invoked when a valid
video file list exists.
In `@media_manager/torrent/schemas.py`:
- Around line 25-27: get_label currently returns
QualityStrings[quality.name].value but QualityStrings.sd still has value "400p",
causing SD to be labeled incorrectly; fix by updating the QualityStrings enum so
the sd member value is "480p" (i.e., change QualityStrings.sd from "400p" to
"480p") or, alternatively, modify get_label in the get_label method to
special-case Quality.sd and return the correct "480p" label from the appropriate
QualityStrings member; reference get_label, Quality, and QualityStrings.sd when
making the change.
In `@media_manager/tv/schemas.py`:
- Line 26: The Episode Pydantic schema declares a persistent-looking field
quality that has no DB column; instead of faking persistence, remove quality
from the Episode schema in media_manager/tv/schemas.py (the Episode class) and
stop mutating the schema inside import_season; in tv/service.py's import_season
flow, compute a local variable (e.g., episode_quality) where you currently set
episode.quality and pass that value into EpisodeFile creation (the code path
that creates an EpisodeFile around where episode is used to build the
EpisodeFile), and update any other usages to read the local variable rather than
expecting Episode.quality to persist.
In `@media_manager/tv/service.py`:
- Around line 663-666: The code accesses video_files[0] without checking for an
empty list which can raise IndexError; update the logic around
extract_quality_video_file and new_path_suffix to first check that video_files
is non-empty (refer to the video_files variable and the call
extract_quality_video_file(video_files[0])), and if empty either set a sensible
default for episode.quality (or skip quality extraction) before calling
QualityStrings.get_label(episode.quality) and building new_path_suffix with
file_path_suffix so the code handles the empty-list case safely.
In `@media_manager/utils/file_handler.py`:
- Around line 69-80: The archive_types set in file_handler.py contains an
impossible combined MIME string
"application/x-zip-compressedapplication/x-compressed"; update the archive_types
variable to split that into two separate entries ("application/x-zip-compressed"
and "application/x-compressed") so files reported with either MIME are
recognized and extracted; keep all other MIME strings unchanged.
- Around line 36-50: The regex match for quality tokens is lowercased but not
normalised for separators, so tokens like "ultra hd", "ultra-hd", "full hd" etc.
fall through; update the code after match = re.search(...) to normalise the
captured token (e.g., take match.group(1), lower() it and strip/replace
separators such as spaces, dots, underscores and hyphens) into a canonical form
before the branching checks used in the quality mapping (refer to
quality_pattern, match, quality and the subsequent if checks returning
Quality.uhd/Quality.fullhd/Quality.hd/Quality.sd) so variants like "ultra-hd"
and "full hd" map correctly.
- Around line 170-177: The current regex in subtitle_pattern makes the separator
optional, causing files like "Movie.srt" to be mis-parsed as a language; change
the pattern so the separator before the language code is required (make the
leading `[. ]?` mandatory, e.g., `[.\s]` or similar) so regex_result only
matches when there is an explicit separator and language code; keep the same
groups (group(1) for language_code and group(2) for variant) and leave the
downstream logic using language_code, target_subtitle_file, and import_file
unchanged.
---
Outside diff comments:
In `@media_manager/torrent/service.py`:
- Around line 59-67: cancel_download currently removes the torrent from the
download manager but does not persist any change to the torrent record, leaving
a stale DB row; after calling self.download_manager.remove_torrent(torrent,
delete_data=delete_files) either set torrent.status = "cancelled" (or "removed")
and persist that change via the existing persistence API (e.g.,
self.torrent_repository.update / save) or, if callers expect immediate deletion,
call the existing delete_torrent(torrent) helper here; update cancel_download to
persist the status change or delete the record so callers in other services
(which raise after calling cancel_download) cannot leave stale rows.
In `@media_manager/tv/service.py`:
- Around line 1017-1027: The EpisodeFile record is created with
file_path_suffix="IMPORTED" but import_season builds filenames as
f"{QualityStrings.get_label(episode.quality)} - {file_path_suffix}", causing
mismatch; fix by ensuring the same computed suffix is used when persisting:
either have import_season return the computed new_path_suffix and use that value
when constructing EpisodeFile, or compute new_path_suffix locally before calling
import_season (using QualityStrings.get_label(episode.quality) + " - " +
file_path_suffix) and pass that exact string into EpisodeFile (used by
tv_repository.add_episode_file) so stored file_path_suffix matches the actual
filename.
---
Nitpick comments:
In `@media_manager/tv/service.py`:
- Around line 678-700: The except handlers for FileNotFoundError and generic
Exception in the import block are asymmetric: FileNotFoundError is logged and
re-raised while other exceptions are logged, notify via
notification_service.send_notification_to_all_providers, and only set success =
False, which causes different control flow for missing files vs other errors.
Decide and implement consistent behavior: either stop the import for all errors
(remove success = False and re-raise in the generic Exception handler) or treat
missing files as non-fatal (catch FileNotFoundError, log/notify and set success
= False instead of re-raising); alternatively, if the current asymmetry is
intentional, add a brief clarifying comment above the FileNotFoundError handler
describing why missing files must fail-fast. Reference the FileNotFoundError
except block, the generic except Exception block, log.exception calls, the
success flag, and notification_service.send_notification_to_all_providers when
making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3ee0fbae-d67d-484d-a263-4670870f306e
📒 Files selected for processing (13)
.gitattributesmedia_manager/indexer/schemas.pymedia_manager/indexer/service.pymedia_manager/movies/router.pymedia_manager/movies/service.pymedia_manager/torrent/schemas.pymedia_manager/torrent/service.pymedia_manager/torrent/utils.pymedia_manager/tv/router.pymedia_manager/tv/schemas.pymedia_manager/tv/service.pymedia_manager/utils/__init__.pymedia_manager/utils/file_handler.py
💤 Files with no reviewable changes (1)
- media_manager/torrent/utils.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-push (arm64, ubuntu-24.04-arm)
🔇 Additional comments (5)
media_manager/movies/service.py (2)
30-45: LGTM!Import changes correctly bring in
QualityStringsfor quality label mapping and the newfile_handlerutilities (extract_quality_video_file,import_subtitle) used in the refactored import logic.
470-475: LGTM!Subtitle import logic is now delegated to the centralised
import_subtitleutility, which should handle language code extraction (including 3-character codes per issue#475) consistently across movie and TV imports.media_manager/tv/service.py (3)
625-639: LGTM!The refactored
import_episodecorrectly uses the centralisedimport_subtitleutility and has improved error handling with specificFileNotFoundErrorcatching. The fail-fast behaviour on file errors is reasonable for import operations.
22-54: LGTM!Import changes correctly consolidate file handling utilities from the new
file_handlermodule and addQualityStringsfor quality label mapping, consistent with the refactoring inmovies/service.py.
723-725: LGTM!Subtitle import in
import_episode_filescorrectly delegates to the centralisedimport_subtitleutility.
| file_quality = extract_quality_video_file(video_files[0]) | ||
| file_suffix = f"{QualityStrings.get_label(file_quality)} - IMPORTED" | ||
|
|
||
| success = self.import_movie( | ||
| movie=movie, | ||
| video_files=video_files, | ||
| subtitle_files=subtitle_files, | ||
| file_path_suffix="IMPORTED", | ||
| file_path_suffix=file_suffix, | ||
| ) |
There was a problem hiding this comment.
Potential IndexError if no video files found in source directory.
video_files[0] will raise an IndexError if get_files_for_import returns an empty list. Unlike import_torrent_files (line 488) which validates len(video_files) != 1, this method lacks a guard.
🛡️ Proposed fix to add validation
video_files, subtitle_files, _all_files = get_files_for_import(
directory=new_source_path
)
+ if not video_files:
+ log.error(f"No video files found in {new_source_path}")
+ return False
+
file_quality = extract_quality_video_file(video_files[0])
file_suffix = f"{QualityStrings.get_label(file_quality)} - IMPORTED"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| file_quality = extract_quality_video_file(video_files[0]) | |
| file_suffix = f"{QualityStrings.get_label(file_quality)} - IMPORTED" | |
| success = self.import_movie( | |
| movie=movie, | |
| video_files=video_files, | |
| subtitle_files=subtitle_files, | |
| file_path_suffix="IMPORTED", | |
| file_path_suffix=file_suffix, | |
| ) | |
| video_files, subtitle_files, _all_files = get_files_for_import( | |
| directory=new_source_path | |
| ) | |
| if not video_files: | |
| log.error(f"No video files found in {new_source_path}") | |
| return False | |
| file_quality = extract_quality_video_file(video_files[0]) | |
| file_suffix = f"{QualityStrings.get_label(file_quality)} - IMPORTED" | |
| success = self.import_movie( | |
| movie=movie, | |
| video_files=video_files, | |
| subtitle_files=subtitle_files, | |
| file_path_suffix=file_suffix, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@media_manager/movies/service.py` around lines 568 - 576, The code calls
extract_quality_video_file(video_files[0]) without checking that video_files is
non-empty, which can raise IndexError; update the surrounding logic in the
method to validate video_files (from get_files_for_import) before using
video_files[0], e.g., check len(video_files) > 0 (or handle len(video_files) !=
1 if that is the intended invariant), and if empty log or raise a clear
exception or return early instead of proceeding to call
extract_quality_video_file and import_movie; make the check near where
file_quality is computed and ensure import_movie is only invoked when a valid
video file list exists.
| @staticmethod | ||
| def get_label(quality: Quality) -> str: | ||
| return QualityStrings[quality.name].value |
There was a problem hiding this comment.
get_label() now surfaces the wrong SD label.
Line 27 returns QualityStrings.sd.value, but that enum member is still "400p". Both quality parsers map 480p to Quality.sd, so this helper will label SD imports inconsistently.
Proposed fix
class QualityStrings(Enum):
uhd = "4K"
fullhd = "1080p"
hd = "720p"
- sd = "400p"
+ sd = "480p"
unknown = "unknown"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@media_manager/torrent/schemas.py` around lines 25 - 27, get_label currently
returns QualityStrings[quality.name].value but QualityStrings.sd still has value
"400p", causing SD to be labeled incorrectly; fix by updating the QualityStrings
enum so the sd member value is "480p" (i.e., change QualityStrings.sd from
"400p" to "480p") or, alternatively, modify get_label in the get_label method to
special-case Quality.sd and return the correct "480p" label from the appropriate
QualityStrings member; reference get_label, Quality, and QualityStrings.sd when
making the change.
| external_id: int | ||
| title: str | ||
| overview: str | None = None | ||
| quality: Quality = Quality.unknown |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check Episode ORM model for quality field
rg -n "quality" media_manager/tv/models.py
# Check for any migration adding quality to episode table
rg -n "episode.*quality|quality.*episode" alembic/versions/Repository: maxdorninger/MediaManager
Length of output: 728
🏁 Script executed:
# Get context around line 79 in models.py to understand the Episode model structure
sed -n '50,100p' media_manager/tv/models.py | cat -n
# Also check the full Episode class definition
ast-grep --pattern $'class Episode($_) {
$$$
}'Repository: maxdorninger/MediaManager
Length of output: 1506
🏁 Script executed:
# Check tv/service.py around the lines mentioned (663 and 1022)
sed -n '660,670p' media_manager/tv/service.py | cat -n
sed -n '1015,1030p' media_manager/tv/service.py | cat -n
# Check migration files for Episode table structure
fd -e py alembic/versions/ | head -5 | xargs grep -l "episode" | xargs -I {} sh -c 'echo "=== {} ===" && head -50 {}'Repository: maxdorninger/MediaManager
Length of output: 1701
🏁 Script executed:
# List migration files in the correct directory
ls -la alembic/versions/ | head -20
# Look for the initial Episode table creation
rg -l "create_table.*episode|episode.*create_table" alembic/versions/
# Check migration files more carefully for Episode table definition
fd . alembic/versions/ -e py | xargs grep -l "episode" | head -5Repository: maxdorninger/MediaManager
Length of output: 2021
🏁 Script executed:
# Check initial migration for Episode table definition
rg -A 20 "create_table.*episode" alembic/versions/93fb07842385_initial_migration.py
# Check if any migration modifies Episode table to add quality
rg "episode.*quality|ALTER TABLE.*episode" alembic/versions/Repository: maxdorninger/MediaManager
Length of output: 684
🏁 Script executed:
# Read the initial migration file to see Episode table structure
cat alembic/versions/93fb07842385_initial_migration.py | grep -A 30 "episode"Repository: maxdorninger/MediaManager
Length of output: 2008
Episode schema includes quality field without database backing.
The Episode Pydantic schema declares a quality field, but the corresponding ORM model and database table do not contain this column. The initial migration creates the episode table with only id, season_id, number, external_id, and title fields.
This means instances loaded from the database via from_attributes=True will always have quality=Quality.unknown, and any value set on the schema object will not be persisted.
Current usage in tv/service.py treats this as transient storage (set at line 664, used at line 1022 to create EpisodeFile), which functions correctly but creates a misleading schema that suggests persistence where none exists.
Either add the quality column to the Episode model and a migration, or use a local variable in import_season instead of mutating the schema object.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@media_manager/tv/schemas.py` at line 26, The Episode Pydantic schema declares
a persistent-looking field quality that has no DB column; instead of faking
persistence, remove quality from the Episode schema in
media_manager/tv/schemas.py (the Episode class) and stop mutating the schema
inside import_season; in tv/service.py's import_season flow, compute a local
variable (e.g., episode_quality) where you currently set episode.quality and
pass that value into EpisodeFile creation (the code path that creates an
EpisodeFile around where episode is used to build the EpisodeFile), and update
any other usages to read the local variable rather than expecting
Episode.quality to persist.
| episode.quality = extract_quality_video_file(video_files[0]) | ||
| new_path_suffix = ( | ||
| f"{QualityStrings.get_label(episode.quality)} - {file_path_suffix}" | ||
| ) |
There was a problem hiding this comment.
Potential IndexError if no video files provided.
Same issue as in movies/service.py: video_files[0] will raise IndexError if the list is empty. Consider adding a guard before accessing the first element.
🛡️ Proposed fix
for episode in season.episodes:
try:
+ if not video_files:
+ log.error(f"No video files available for S{season.number:02d}E{episode.number:02d}")
+ success = False
+ continue
episode.quality = extract_quality_video_file(video_files[0])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@media_manager/tv/service.py` around lines 663 - 666, The code accesses
video_files[0] without checking for an empty list which can raise IndexError;
update the logic around extract_quality_video_file and new_path_suffix to first
check that video_files is non-empty (refer to the video_files variable and the
call extract_quality_video_file(video_files[0])), and if empty either set a
sensible default for episode.quality (or skip quality extraction) before calling
QualityStrings.get_label(episode.quality) and building new_path_suffix with
file_path_suffix so the code handles the empty-list case safely.
| quality_pattern = ( | ||
| r"\b(4k|UHD|ultra[ ._-]?hd|2160p|FHD|full[ ._-]?hd|1080p|HD|720p|SD|480p)\b" | ||
| ) | ||
| match = re.search(quality_pattern, file.stem, re.IGNORECASE) | ||
| if match: | ||
| quality = match.group(1).lower() | ||
| if quality in {"4k", "uhd", "2160p"}: | ||
| return Quality.uhd | ||
| if quality in {"fhd", "1080p"}: | ||
| return Quality.fullhd | ||
| if quality in {"hd", "720p"}: | ||
| return Quality.hd | ||
| if quality in {"sd", "480p"}: | ||
| return Quality.sd | ||
| return Quality.unknown |
There was a problem hiding this comment.
Normalise matched quality tokens before branching.
Line 41 lowercases the regex match but does not normalise separators, so ultra hd, ultra-hd, full hd, and full-hd still fall through to Quality.unknown.
Proposed fix
match = re.search(quality_pattern, file.stem, re.IGNORECASE)
if match:
- quality = match.group(1).lower()
- if quality in {"4k", "uhd", "2160p"}:
+ quality = re.sub(r"[ ._-]+", "", match.group(1).lower())
+ if quality in {"4k", "uhd", "ultrahd", "2160p"}:
return Quality.uhd
- if quality in {"fhd", "1080p"}:
+ if quality in {"fhd", "fullhd", "1080p"}:
return Quality.fullhd
if quality in {"hd", "720p"}:
return Quality.hd
if quality in {"sd", "480p"}:
return Quality.sd🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@media_manager/utils/file_handler.py` around lines 36 - 50, The regex match
for quality tokens is lowercased but not normalised for separators, so tokens
like "ultra hd", "ultra-hd", "full hd" etc. fall through; update the code after
match = re.search(...) to normalise the captured token (e.g., take
match.group(1), lower() it and strip/replace separators such as spaces, dots,
underscores and hyphens) into a canonical form before the branching checks used
in the quality mapping (refer to quality_pattern, match, quality and the
subsequent if checks returning Quality.uhd/Quality.fullhd/Quality.hd/Quality.sd)
so variants like "ultra-hd" and "full hd" map correctly.
| archive_types = { | ||
| "application/zip", | ||
| "application/x-zip-compressedapplication/x-compressed", | ||
| "application/vnd.rar", | ||
| "application/x-7z-compressed", | ||
| "application/x-freearc", | ||
| "application/x-bzip", | ||
| "application/x-bzip2", | ||
| "application/gzip", | ||
| "application/x-gzip", | ||
| "application/x-tar", | ||
| } |
There was a problem hiding this comment.
Split the ZIP MIME aliases into separate entries.
Line 71 is one impossible MIME string, so files reported as application/x-zip-compressed or application/x-compressed will never be extracted.
Proposed fix
archive_types = {
"application/zip",
- "application/x-zip-compressedapplication/x-compressed",
+ "application/x-zip-compressed",
+ "application/x-compressed",
"application/vnd.rar",
"application/x-7z-compressed",
"application/x-freearc",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| archive_types = { | |
| "application/zip", | |
| "application/x-zip-compressedapplication/x-compressed", | |
| "application/vnd.rar", | |
| "application/x-7z-compressed", | |
| "application/x-freearc", | |
| "application/x-bzip", | |
| "application/x-bzip2", | |
| "application/gzip", | |
| "application/x-gzip", | |
| "application/x-tar", | |
| } | |
| archive_types = { | |
| "application/zip", | |
| "application/x-zip-compressed", | |
| "application/x-compressed", | |
| "application/vnd.rar", | |
| "application/x-7z-compressed", | |
| "application/x-freearc", | |
| "application/x-bzip", | |
| "application/x-bzip2", | |
| "application/gzip", | |
| "application/x-gzip", | |
| "application/x-tar", | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@media_manager/utils/file_handler.py` around lines 69 - 80, The archive_types
set in file_handler.py contains an impossible combined MIME string
"application/x-zip-compressedapplication/x-compressed"; update the archive_types
variable to split that into two separate entries ("application/x-zip-compressed"
and "application/x-compressed") so files reported with either MIME are
recognized and extracted; keep all other MIME strings unchanged.
| subtitle_pattern = r"[. ]?([a-z]{2,3})(?:\.(\d+))?\.srt$" | ||
| regex_result = re.search(subtitle_pattern, subtitle_file.name, re.IGNORECASE) | ||
| if regex_result: | ||
| language_code = regex_result.group(1) | ||
| if regex_result.group(2): | ||
| language_code += f".{regex_result.group(2)}" | ||
| target_subtitle_file = target_file.with_suffix(f".{language_code}.srt") | ||
| import_file(target_file=target_subtitle_file, source_file=subtitle_file) |
There was a problem hiding this comment.
Require an explicit separator before parsing a language code.
Line 170 makes [. ] optional, so plain subtitles such as Movie.srt or Cars.srt are parsed as fake language codes (vie, ars) and renamed incorrectly. That regresses existing .srt imports while fixing 3-character codes.
Proposed fix
- subtitle_pattern = r"[. ]?([a-z]{2,3})(?:\.(\d+))?\.srt$"
+ subtitle_pattern = r"(?:[. ]([a-z]{2,3})(?:\.(\d+))?)?\.srt$"
regex_result = re.search(subtitle_pattern, subtitle_file.name, re.IGNORECASE)
if regex_result:
- language_code = regex_result.group(1)
- if regex_result.group(2):
- language_code += f".{regex_result.group(2)}"
- target_subtitle_file = target_file.with_suffix(f".{language_code}.srt")
+ language_code = regex_result.group(1)
+ subtitle_index = regex_result.group(2)
+ if language_code is None:
+ target_subtitle_file = target_file.with_suffix(".srt")
+ else:
+ if subtitle_index:
+ language_code += f".{subtitle_index}"
+ target_subtitle_file = target_file.with_suffix(f".{language_code}.srt")
import_file(target_file=target_subtitle_file, source_file=subtitle_file)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@media_manager/utils/file_handler.py` around lines 170 - 177, The current
regex in subtitle_pattern makes the separator optional, causing files like
"Movie.srt" to be mis-parsed as a language; change the pattern so the separator
before the language code is required (make the leading `[. ]?` mandatory, e.g.,
`[.\s]` or similar) so regex_result only matches when there is an explicit
separator and language code; keep the same groups (group(1) for language_code
and group(2) for variant) and leave the downstream logic using language_code,
target_subtitle_file, and import_file unchanged.
1 participant
Summary
So we can say that close #315 #444 (although the error is not related to the issue's title) and fixes #475
Summary by CodeRabbit
Release Notes
New Features
Improvements
Chores