refactor(BJS): simplify search_existing using direct HTML parsing - #1378
refactor(BJS): simplify search_existing using direct HTML parsing#1378wastaken7 wants to merge 2 commits into
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. |
📝 WalkthroughWalkthroughRefactors the BJS tracker: removes an unused import, updates get_database_title() to parse the "Informações" box for original/canonical titles, simplifies search_existing() to build duplicates from the search-results page rows, and adjusts get_year() to prefer tvdb/imdb year values for TV or meta year for movies. ChangesBJS Tracker Refactoring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/trackers/BJS.py (1)
476-521:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't turn search failures into a clean “no dupes” result.
This path now returns
[]for HTTP failures, auth regressions, and unexpected HTML without settingmeta["skipping"]. Insrc/trackerstatus.py, that still flows through the normal dupe-check pipeline, so a broken BJS search can be mistaken for “safe to upload”.Suggested fix
try: cookie_jar = await self.cookie_validator.load_session_cookies(meta, self.tracker) if cookie_jar: self.session.cookies = cookie_jar @@ search_url = f"{self.base_url}/torrents.php?searchstr={meta['imdb_info']['imdbID']}" response = await self.session.get(search_url, follow_redirects=True) + response.raise_for_status() soup = BeautifulSoup(response.text, "html.parser") torrent_details_table: Optional[Tag] = soup.find("div", class_="main_column") @@ if torrent_details_table: BJS.database_title = self.get_database_title(soup) BJS.already_has_the_info = bool(BJS.database_title) else: + meta["skipping"] = self.tracker return dupes @@ except Exception as e: + meta["skipping"] = self.tracker console.print(f'[bold red]Ocorreu um erro inesperado ao processar a busca: {e}[/bold red]') import traceback traceback.print_exc() return dupes🤖 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/trackers/BJS.py` around lines 476 - 521, The function currently swallows HTTP/parser/auth failures and returns an empty dupes list, which makes failures look like “no dupes”; update error handling so that on non-200 HTTP responses, missing/invalid HTML (when torrent_details_table is falsy), or any exception you set meta["skipping"] = True (to mark the search as failed) and then either return immediately or re-raise after setting that flag; specifically change the paths around cookie_validator.load_session_cookies, the response handling after self.session.get(...), the branch that returns when torrent_details_table is falsy, and the except block so they set meta["skipping"]=True before returning or propagating the error (references: cookie_validator.load_session_cookies, self.session.get, torrent_details_table, BJS.already_has_the_info, get_database_title, meta["skipping"]).
🤖 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/trackers/BJS.py`:
- Around line 488-493: The code currently sets BJS.already_has_the_info whenever
a generic "div.main_column" exists; change this so the flag is only set when the
page actually contains an info entry by verifying the real metadata before
marking it. Concretely, keep locating torrent_details_table = soup.find("div",
class_="main_column") but only set BJS.already_has_the_info = True and assign
BJS.database_title = self.get_database_title(soup) if get_database_title(soup)
returns a non-empty/truthy value (or if torrent_details_table contains a more
specific expected element indicating a BJS entry); otherwise leave
already_has_the_info False so downstream methods (get_cover, get_credits,
get_overview) don’t skip fields for search-result pages.
---
Outside diff comments:
In `@src/trackers/BJS.py`:
- Around line 476-521: The function currently swallows HTTP/parser/auth failures
and returns an empty dupes list, which makes failures look like “no dupes”;
update error handling so that on non-200 HTTP responses, missing/invalid HTML
(when torrent_details_table is falsy), or any exception you set meta["skipping"]
= True (to mark the search as failed) and then either return immediately or
re-raise after setting that flag; specifically change the paths around
cookie_validator.load_session_cookies, the response handling after
self.session.get(...), the branch that returns when torrent_details_table is
falsy, and the except block so they set meta["skipping"]=True before returning
or propagating the error (references: cookie_validator.load_session_cookies,
self.session.get, torrent_details_table, BJS.already_has_the_info,
get_database_title, meta["skipping"]).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| torrent_details_table: Optional[Tag] = soup.find("div", class_="main_column") | ||
|
|
||
| if torrent_details_table: | ||
| BJS.already_has_the_info = True | ||
| BJS.database_title = self.get_database_title(soup) | ||
| else: |
There was a problem hiding this comment.
Only set already_has_the_info when the info box was actually found.
div.main_column is the generic search-results container, so Line 491 currently marks the tracker as already having metadata even when this page only contains a search listing. Downstream, get_cover(), get_credits(), and get_overview() treat that flag as authoritative and can skip fields for uploads that do not already exist in BJS's database.
Suggested fix
torrent_details_table: Optional[Tag] = soup.find("div", class_="main_column")
if torrent_details_table:
- BJS.already_has_the_info = True
BJS.database_title = self.get_database_title(soup)
+ BJS.already_has_the_info = bool(BJS.database_title)
else:
return dupes🤖 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/trackers/BJS.py` around lines 488 - 493, The code currently sets
BJS.already_has_the_info whenever a generic "div.main_column" exists; change
this so the flag is only set when the page actually contains an info entry by
verifying the real metadata before marking it. Concretely, keep locating
torrent_details_table = soup.find("div", class_="main_column") but only set
BJS.already_has_the_info = True and assign BJS.database_title =
self.get_database_title(soup) if get_database_title(soup) returns a
non-empty/truthy value (or if torrent_details_table contains a more specific
expected element indicating a BJS entry); otherwise leave already_has_the_info
False so downstream methods (get_cover, get_credits, get_overview) don’t skip
fields for search-result pages.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/trackers/BJS.py (1)
465-482:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReset the shared BJS state before any early return.
BJS.already_has_the_infoandBJS.database_titleare class-level state, but they are only cleared after the subtitle and IMDb guards. If a previous upload populated them, returning early here can leak stale values intoget_title(),get_cover(),get_credits(), andget_overview()on the next upload.Suggested fix
async def search_existing(self, meta: dict[str, Any], _) -> list[dict[str, str]]: dupes: list[dict[str, str]] = [] + BJS.already_has_the_info = False + BJS.database_title = "" + should_continue = self.get_additional_checks(meta) if not should_continue: meta["skipping"] = f"{self.tracker}" return dupes @@ - BJS.already_has_the_info = False - BJS.database_title = "" - search_url = f"{self.base_url}/torrents.php?searchstr={meta['imdb_info']['imdbID']}"🤖 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/trackers/BJS.py` around lines 465 - 482, Reset the class-level state BJS.already_has_the_info and BJS.database_title at the start of search_existing (or at least before any early returns such as the subtitle guard and the IMDb ID guard) to avoid leaking stale values; locate the async def search_existing(...) and move or add assignments to set BJS.already_has_the_info = False and BJS.database_title = "" before the checks that return early (the get_additional_checks() branch and the imdbID presence check) so downstream methods like get_title(), get_cover(), get_credits(), and get_overview() never see stale class state.
🤖 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.
Outside diff comments:
In `@src/trackers/BJS.py`:
- Around line 465-482: Reset the class-level state BJS.already_has_the_info and
BJS.database_title at the start of search_existing (or at least before any early
returns such as the subtitle guard and the IMDb ID guard) to avoid leaking stale
values; locate the async def search_existing(...) and move or add assignments to
set BJS.already_has_the_info = False and BJS.database_title = "" before the
checks that return early (the get_additional_checks() branch and the imdbID
presence check) so downstream methods like get_title(), get_cover(),
get_credits(), and get_overview() never see stale class state.
1 participant
Description
This PR refactors and significantly simplifies the duplicate check logic (
search_existing) for the BJS tracker.Since the tracker now exposes the folder/file names directly on the torrent listing via the
data-torrentnameattribute, we no longer need to perform secondary AJAX requests or fetch individual torrent pages. Furthermore, all unnecessary filtering and metadata matching (resolution, season/episode, pack matching) have been removed, returning all search matches directly.Changes
src/trackers/BJS.py):search_existingto directly extractdata-torrentnameandsizefrom the list rows on the search page._fetch_search_page.name,size,link)._extract_upload_params,_should_process_torrent,_extract_torrent_ids,_fetch_torrent_page,_extract_item_name, and_process_ajax_responses).castfromtyping).Summary by CodeRabbit