fix(FL): Update function edit_desc #1317
Open
vladciobancai wants to merge 3 commits intoAudionut:masterfrom
Open
fix(FL): Update function edit_desc #1317vladciobancai wants to merge 3 commits intoAudionut:masterfrom
vladciobancai wants to merge 3 commits intoAudionut:masterfrom
Conversation
|
Thanks for taking the time to contribute to this project. Upload Assistant is currently in a complete rewrite, and no new development is being conducted on this python source at this time. If you have come this far, please feel free to leave open, any pull requests regarding new sites being added to the source, as these can serve as the baseline for later conversion. If your pull request relates to a critical bug, this will be addressed in this code base, and a new release published as needed. If your pull request only addresses a quite minor bug, it is not likely to be addressed in this code base. Details for the new code base will follow at a later date. |
📝 WalkthroughWalkthroughThe FL tracker configuration and implementation are updated. A new password field is added to the example configuration, fltools credentials are introduced, and the tracker's upload endpoints are modified. Image selection patterns are simplified, and response parsing now uses HTML extraction with BeautifulSoup instead of direct text normalization. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/trackers/FL.py (1)
390-410:⚠️ Potential issue | 🟠 MajorGuard missing
fltoolscredentials before auth usage.
auth=(self.fltools['user'], self.fltools['pass'])can raiseKeyErrorwhen older configs don’t yet include[TRACKERS][FL][fltools], causing a hard crash instead of a clear config error.Proposed fix
+ fltools_user = str(self.fltools.get('user', '')).strip() + fltools_pass = str(self.fltools.get('pass', '')).strip() + if not fltools_user or not fltools_pass: + raise UploadException( + "Missing FL tools credentials: set [TRACKERS][FL][fltools][user/pass] in config", + 'red' + ) # noqa: F405 async with httpx.AsyncClient(timeout=30.0) as client: - response = await client.post(url, data=data, files=files, auth=(self.fltools['user'], self.fltools['pass'])) + response = await client.post(url, data=data, files=files, auth=(fltools_user, fltools_pass)) @@ + fltools_user = str(self.fltools.get('user', '')).strip() + fltools_pass = str(self.fltools.get('pass', '')).strip() + if not fltools_user or not fltools_pass: + raise UploadException( + "Missing FL tools credentials: set [TRACKERS][FL][fltools][user/pass] in config", + 'red' + ) # noqa: F405 async with httpx.AsyncClient(timeout=30.0) as client: - response = await client.post(url, files=files, auth=(self.fltools['user'], self.fltools['pass'])) + response = await client.post(url, files=files, auth=(fltools_user, fltools_pass))Also applies to: 421-434
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/trackers/FL.py` around lines 390 - 410, The code uses auth=(self.fltools['user'], self.fltools['pass']) without checking that self.fltools exists or contains the 'user'/'pass' keys; add a guard before creating the AsyncClient request (the block that builds url, data, files and calls client.post) to validate self.fltools is a dict-like with both 'user' and 'pass' (or raise/return a clear configuration error), e.g., check presence of self.fltools and keys before the client.post call and handle the missing credentials by logging/raising a descriptive error instead of letting a KeyError propagate from the auth tuple construction.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/trackers/FL.py`:
- Line 398: The glob pattern in the screen_glob assignment is too broad and
picks up non-screenshot PNGs; update the glob.glob call (used where screen_glob
is computed for meta['base_dir'] and meta['uuid']) to either narrow the pattern
to the actual screenshot filename convention (e.g., use the screenshot prefix
used elsewhere) or post-filter the results to exclude known non-screenshot
prefixes like "FILE", "PLAYLIST", and "POSTER"; ensure the same change is
applied to the other occurrence that also builds screen_glob.
- Around line 396-397: The imdb gating currently uses meta.get('imdb_id') != 0
which fails for string IDs like "0" and unconditionally appends '/' even when
imdb_info.imdb_url is missing; update the logic (in the code handling meta,
imdb_id, imdb_info and data['imdb_url']) to: 1) treat imdb_id as absent if it's
0, "0", empty or None (e.g., convert to str and check truthiness or explicitly
check both 0 and "0"), and 2) only set data['imdb_url'] when
imdb_info.get('imdb_url') is a non-empty string, adding a trailing '/' only if
the URL exists and doesn't already end with '/'.
---
Outside diff comments:
In `@src/trackers/FL.py`:
- Around line 390-410: The code uses auth=(self.fltools['user'],
self.fltools['pass']) without checking that self.fltools exists or contains the
'user'/'pass' keys; add a guard before creating the AsyncClient request (the
block that builds url, data, files and calls client.post) to validate
self.fltools is a dict-like with both 'user' and 'pass' (or raise/return a clear
configuration error), e.g., check presence of self.fltools and keys before the
client.post call and handle the missing credentials by logging/raising a
descriptive error instead of letting a KeyError propagate from the auth tuple
construction.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9a5c3dd9-4e4a-4028-9c5d-3640fce1554a
📒 Files selected for processing (2)
data/example-config.pysrc/trackers/FL.py
Comment on lines
+396
to
+397
| if meta.get('imdb_id') != 0: | ||
| data['imdb_url'] = str(meta.get('imdb_info', {}).get('imdb_url', '')) + '/' |
There was a problem hiding this comment.
Fix IMDb URL gating to avoid sending malformed imdb_url.
At Line 396, meta.get('imdb_id') != 0 is unsafe for string IDs (e.g., "0"), and Line 397 can send "/" when imdb_info.imdb_url is missing.
Proposed fix
- if meta.get('imdb_id') != 0:
- data['imdb_url'] = str(meta.get('imdb_info', {}).get('imdb_url', '')) + '/'
+ imdb_id_value = str(meta.get('imdb_id', '0')).strip()
+ imdb_info = meta.get('imdb_info')
+ imdb_info_dict = cast(dict[str, Any], imdb_info) if isinstance(imdb_info, dict) else {}
+ imdb_url = str(imdb_info_dict.get('imdb_url', '')).strip()
+ if imdb_id_value.isdigit() and int(imdb_id_value) != 0 and imdb_url:
+ data['imdb_url'] = imdb_url.rstrip('/') + '/'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/trackers/FL.py` around lines 396 - 397, The imdb gating currently uses
meta.get('imdb_id') != 0 which fails for string IDs like "0" and unconditionally
appends '/' even when imdb_info.imdb_url is missing; update the logic (in the
code handling meta, imdb_id, imdb_info and data['imdb_url']) to: 1) treat
imdb_id as absent if it's 0, "0", empty or None (e.g., convert to str and check
truthiness or explicitly check both 0 and "0"), and 2) only set data['imdb_url']
when imdb_info.get('imdb_url') is a non-empty string, adding a trailing '/' only
if the URL exists and doesn't already end with '/'.
Owner
There was a problem hiding this comment.
imdb_id is an int, but the forward slash should be addressed.
There was a problem hiding this comment.
@Audionut, understood — since imdb_id is always an int, the condition is fine as-is. The only issue is the unconditional / append when imdb_url is empty. Here's a minimal fix:
if meta.get('imdb_id') != 0:
imdb_url = str(meta.get('imdb_info', {}).get('imdb_url', '')).strip()
if imdb_url:
data['imdb_url'] = imdb_url.rstrip('/') + '/'This ensures data['imdb_url'] is only set when the URL is actually present, and avoids double slashes if the URL already ends with one.
(ᐢ•ᴗ•ᐢ)∫
✏️ Learnings added
Learnt from: Audionut
Repo: Audionut/Upload-Assistant PR: 1317
File: src/trackers/FL.py:396-397
Timestamp: 2026-03-23T10:00:38.629Z
Learning: In src/trackers/FL.py, meta['imdb_id'] is always an integer. Direct integer comparison (e.g., meta.get('imdb_id') != 0) is safe without needing to convert to string first.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: Audionut
Repo: Audionut/Upload-Assistant PR: 1007
File: src/trackers/RTF.py:38-38
Timestamp: 2025-12-01T01:43:01.865Z
Learning: In src/trackers/RTF.py, the forum_link (https://retroflix.club/forums.php?action=viewtopic&topicid=3619) is intentionally passed as the signature parameter to DescriptionBuilder.unit3d_edit_desc() for historical purposes, which is RTF-specific behavior.
Learnt from: Audionut
Repo: Audionut/Upload-Assistant PR: 1025
File: src/trackers/TVC.py:860-873
Timestamp: 2025-12-09T10:08:42.221Z
Learning: In the Upload-Assistant codebase, metadata dictionary keys with `_id` suffix (e.g., `meta['imdb_id']`, `meta['tmdb_id']`, `meta['tvdb_id']`, `meta['mal_id']`) are integer stores of ID values, while unsuffixed versions like `meta['imdb']` and `meta['tmdb']` may be string versions or used for different purposes such as API data. Code should use the `_id` suffixed versions when checking for presence of integer IDs.
Learnt from: Audionut
Repo: Audionut/Upload-Assistant PR: 1201
File: src/get_desc.py:579-587
Timestamp: 2026-02-02T07:44:14.622Z
Learning: In the Upload-Assistant codebase, meta['tvmaze_episode_data'] is only set when meta['tvmaze_id'] is present; therefore, accessing meta['tvmaze_id'] inside branches guarded by 'tvmaze_episode_data' is safe.
Learnt from: Audionut
Repo: Audionut/Upload-Assistant PR: 1201
File: src/get_desc.py:579-587
Timestamp: 2026-02-02T07:41:52.301Z
Learning: In the Upload-Assistant codebase, meta['tvdb_episode_data'] is only set when meta['tvdb_id'] is present; therefore, accessing meta['tvdb_id'] inside branches guarded by 'tvdb_episode_data' is safe.
Learnt from: Audionut
Repo: Audionut/Upload-Assistant PR: 1025
File: src/trackers/TVC.py:32-86
Timestamp: 2025-12-14T00:20:21.219Z
Learning: In src/trackers/TVC.py, the genres argument passed to get_cat_id() is a string (comma-separated) rather than a list or other type. Remove or avoid code paths that handle list inputs for this parameter; assume and validate string input, adjust type hints if present, and, if possible, add a unit test to ensure a string is passed.
Learnt from: Audionut
Repo: Audionut/Upload-Assistant PR: 1081
File: src/trackers/CBR.py:84-91
Timestamp: 2025-12-27T13:04:08.659Z
Learning: In tracker modules under src/trackers, meta['aka'] is a guaranteed key and can be accessed directly (meta['aka']) without using get(). This improves clarity and small performance overhead. Only rely on this when you are certain the key is always present; if a tracker could ever omit it, add a guard or a clear default to avoid KeyError.
Learnt from: Audionut
Repo: Audionut/Upload-Assistant PR: 1081
File: src/trackers/CBR.py:94-94
Timestamp: 2025-12-27T13:04:22.071Z
Learning: In Python codebases where a dict (e.g., meta) is guaranteed to always include a specific key (e.g., 'tag'), direct dictionary access meta['tag'] is safe and preferred over meta.get('tag') for performance and readability. Ensure the key is indeed always initialized in all code paths; if there's any doubt, consider defensive checks or documentation to avoid KeyError.
Learnt from: Audionut
Repo: Audionut/Upload-Assistant PR: 1114
File: src/manualpackage.py:16-23
Timestamp: 2026-01-11T05:07:09.469Z
Learning: In Python type hints for read-only mappings (e.g., DEFAULT_CONFIG, TRACKER_CONFIG), prefer Mapping[str, Any] over Dict[str, Any] to signal immutability intent. Use from typing import Mapping; annotate with Mapping[str, Any] while allowing concrete dicts at runtime. Do not rely on isinstance(value, dict) for immutability checks; this is a type-hint convention, not a runtime guarantee.
Learnt from: Audionut
Repo: Audionut/Upload-Assistant PR: 1114
File: src/manualpackage.py:16-23
Timestamp: 2026-01-11T05:07:09.469Z
Learning: In the Upload-Assistant repository, avoid flagging minor or stylistic issues (e.g., ValueError vs TypeError for type validation) unless they represent actual bugs or significant problems. Follow Audionut's preference to focus on substantive issues rather than minor concerns across Python files.
Learnt from: wasserrutschentester
Repo: Audionut/Upload-Assistant PR: 1250
File: src/trackers/RHD.py:269-271
Timestamp: 2026-02-13T20:15:30.786Z
Learning: In src/trackers/RHD.py, the 'GERMAN SUBBED' tag is a standalone tag that overwrites audio_lang_str when German subtitles exist without German audio. It indicates 'OV (original version) audio + German subtitles' according to RocketHD naming conventions, so the audio language information should be overwritten, not appended. Apply this rule consistently to similar trackers and ensure the tag replaces existing language metadata rather than augmenting it.
| screen_glob = [os.path.basename(f) for f in glob.glob(os.path.join(f"{meta['base_dir']}/tmp/{meta['uuid']}", f"{meta['filename']}-*.png"))] | ||
| if meta.get('imdb_id') != 0: | ||
| data['imdb_url'] = str(meta.get('imdb_info', {}).get('imdb_url', '')) + '/' | ||
| screen_glob = [os.path.basename(f) for f in glob.glob(os.path.join(f"{meta['base_dir']}/tmp/{meta['uuid']}", "*.png"))] |
There was a problem hiding this comment.
*.png glob is too broad and may upload non-screenshot assets.
Using *.png in the temp folder can include files like FILE*, PLAYLIST*, or POSTER*, which are explicitly filtered elsewhere in the codebase.
Proposed fix
- screen_glob = [os.path.basename(f) for f in glob.glob(os.path.join(f"{meta['base_dir']}/tmp/{meta['uuid']}", "*.png"))]
+ all_png = [os.path.basename(f) for f in glob.glob(os.path.join(f"{meta['base_dir']}/tmp/{meta['uuid']}", "*.png"))]
+ unwanted_prefixes = ("FILE", "PLAYLIST", "POSTER")
+ screen_glob = [name for name in all_png if not name.upper().startswith(unwanted_prefixes)]
@@
- screen_glob = [os.path.basename(f) for f in glob.glob(os.path.join(f"{meta['base_dir']}/tmp/{meta['uuid']}", "*.png"))]
+ all_png = [os.path.basename(f) for f in glob.glob(os.path.join(f"{meta['base_dir']}/tmp/{meta['uuid']}", "*.png"))]
+ unwanted_prefixes = ("FILE", "PLAYLIST", "POSTER")
+ screen_glob = [name for name in all_png if not name.upper().startswith(unwanted_prefixes)]Also applies to: 422-422
🧰 Tools
🪛 Ruff (0.15.6)
[error] 398-398: Probable insecure usage of temporary file or directory: "/tmp/"
(S108)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/trackers/FL.py` at line 398, The glob pattern in the screen_glob
assignment is too broad and picks up non-screenshot PNGs; update the glob.glob
call (used where screen_glob is computed for meta['base_dir'] and meta['uuid'])
to either narrow the pattern to the actual screenshot filename convention (e.g.,
use the screenshot prefix used elsewhere) or post-filter the results to exclude
known non-screenshot prefixes like "FILE", "PLAYLIST", and "POSTER"; ensure the
same change is applied to the other occurrence that also builds screen_glob.
Owner
There was a problem hiding this comment.
yes, see
Upload-Assistant/src/uploadscreens.py
Line 692 in 105a92f
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look at the screenshot naming convention around line 692 in uploadscreens.py
sed -n '680,710p' src/uploadscreens.py
Length of output: 1342
@Audionut, thanks for the reference. The uploadscreens.py approach uses *.png followed by explicit exclusion of FILE*, PLAYLIST*, and POSTER*. The same filtering should be applied to both screen_glob assignments in FL.py. Here's a fix consistent with that pattern:
# Non-BDMV (line ~398)
all_png = glob.glob(os.path.join(f"{meta['base_dir']}/tmp/{meta['uuid']}", "*.png"))
unwanted_patterns = ["FILE*", "PLAYLIST*", "POSTER*"]
unwanted_files: set[str] = set()
for pattern in unwanted_patterns:
unwanted_files.update(
os.path.basename(f) for f in glob.glob(os.path.join(f"{meta['base_dir']}/tmp/{meta['uuid']}", pattern))
)
screen_glob = [os.path.basename(f) for f in all_png if os.path.basename(f) not in unwanted_files]# BDMV (line ~422)
all_png = glob.glob(os.path.join(f"{meta['base_dir']}/tmp/{meta['uuid']}", "*.png"))
unwanted_patterns = ["FILE*", "PLAYLIST*", "POSTER*"]
unwanted_files: set[str] = set()
for pattern in unwanted_patterns:
unwanted_files.update(
os.path.basename(f) for f in glob.glob(os.path.join(f"{meta['base_dir']}/tmp/{meta['uuid']}", pattern))
)
screen_glob = [os.path.basename(f) for f in all_png if os.path.basename(f) not in unwanted_files]This mirrors the uploadscreens.py logic while adapting it to work with the full path prefix used in FL.py.
(ᵔᴥᵔ)
🧠 Learnings used
Learnt from: Audionut
Repo: Audionut/Upload-Assistant PR: 1317
File: src/trackers/FL.py:396-397
Timestamp: 2026-03-23T10:00:38.629Z
Learning: In src/trackers/FL.py, meta['imdb_id'] is always an integer. Direct integer comparison (e.g., meta.get('imdb_id') != 0) is safe without needing to convert to string first.
Learnt from: Audionut
Repo: Audionut/Upload-Assistant PR: 1007
File: src/trackers/RTF.py:38-38
Timestamp: 2025-12-01T01:43:01.865Z
Learning: In src/trackers/RTF.py, the forum_link (https://retroflix.club/forums.php?action=viewtopic&topicid=3619) is intentionally passed as the signature parameter to DescriptionBuilder.unit3d_edit_desc() for historical purposes, which is RTF-specific behavior.
Learnt from: Lusephur
Repo: Audionut/Upload-Assistant PR: 1023
File: src/trackers/TVC.py:484-487
Timestamp: 2025-11-23T15:26:45.408Z
Learning: In the Upload-Assistant codebase, particularly for src/trackers/TVC.py, do not mark error handling suggestions as critical severity when the code has been tested and is working successfully across multiple scenarios. The user (Lusephur) has thoroughly tested TVC uploads including TV episodes, TV packs, foreign content, and movies.
Learnt from: Audionut
Repo: Audionut/Upload-Assistant PR: 1114
File: src/manualpackage.py:16-23
Timestamp: 2026-01-11T05:07:09.469Z
Learning: In the Upload-Assistant repository, avoid flagging minor or stylistic issues (e.g., ValueError vs TypeError for type validation) unless they represent actual bugs or significant problems. Follow Audionut's preference to focus on substantive issues rather than minor concerns across Python files.
Learnt from: Audionut
Repo: Audionut/Upload-Assistant PR: 991
File: src/trackers/THR.py:263-263
Timestamp: 2025-11-11T08:09:03.403Z
Learning: For src/trackers/THR.py in Audionut's pull requests, do not flag issues about mixing synchronous requests calls (like requests.post, requests.get) with async functions. The file needs a refactor, and these issues should be ignored until then.
Learnt from: Audionut
Repo: Audionut/Upload-Assistant PR: 1118
File: upload.py:1181-1181
Timestamp: 2026-01-11T06:59:20.992Z
Learning: In the Upload-Assistant codebase, particularly in upload.py around line 1181, the hardcoded `successful_trackers = 10` before the optional secondary dupe check (dupe_again) is intentional design. The real threshold check happens earlier (~line 377), and this value ensures uploads proceed when no secondary check is performed. It gets overwritten when dupe_again is True.
Learnt from: Audionut
Repo: Audionut/Upload-Assistant PR: 1025
File: src/trackers/TVC.py:32-86
Timestamp: 2025-12-14T00:20:21.219Z
Learning: In src/trackers/TVC.py, the genres argument passed to get_cat_id() is a string (comma-separated) rather than a list or other type. Remove or avoid code paths that handle list inputs for this parameter; assume and validate string input, adjust type hints if present, and, if possible, add a unit test to ensure a string is passed.
Learnt from: Audionut
Repo: Audionut/Upload-Assistant PR: 1081
File: src/trackers/CBR.py:84-91
Timestamp: 2025-12-27T13:04:08.659Z
Learning: In tracker modules under src/trackers, meta['aka'] is a guaranteed key and can be accessed directly (meta['aka']) without using get(). This improves clarity and small performance overhead. Only rely on this when you are certain the key is always present; if a tracker could ever omit it, add a guard or a clear default to avoid KeyError.
Learnt from: Audionut
Repo: Audionut/Upload-Assistant PR: 1081
File: src/trackers/CBR.py:94-94
Timestamp: 2025-12-27T13:04:22.071Z
Learning: In Python codebases where a dict (e.g., meta) is guaranteed to always include a specific key (e.g., 'tag'), direct dictionary access meta['tag'] is safe and preferred over meta.get('tag') for performance and readability. Ensure the key is indeed always initialized in all code paths; if there's any doubt, consider defensive checks or documentation to avoid KeyError.
Learnt from: Audionut
Repo: Audionut/Upload-Assistant PR: 1114
File: src/manualpackage.py:16-23
Timestamp: 2026-01-11T05:07:09.469Z
Learning: In Python type hints for read-only mappings (e.g., DEFAULT_CONFIG, TRACKER_CONFIG), prefer Mapping[str, Any] over Dict[str, Any] to signal immutability intent. Use from typing import Mapping; annotate with Mapping[str, Any] while allowing concrete dicts at runtime. Do not rely on isinstance(value, dict) for immutability checks; this is a type-hint convention, not a runtime guarantee.
Learnt from: wasserrutschentester
Repo: Audionut/Upload-Assistant PR: 1250
File: src/trackers/RHD.py:269-271
Timestamp: 2026-02-13T20:15:30.786Z
Learning: In src/trackers/RHD.py, the 'GERMAN SUBBED' tag is a standalone tag that overwrites audio_lang_str when German subtitles exist without German audio. It indicates 'OV (original version) audio + German subtitles' according to RocketHD naming conventions, so the audio language information should be overwritten, not appended. Apply this rule consistently to similar trackers and ensure the tag replaces existing language metadata rather than augmenting it.
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
2 participants
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 by CodeRabbit
Configuration Updates
Improvements