Provider warning #8816
Provider warning #8816
Conversation
WalkthroughThe pull request introduces enhancements to the logging functionality across multiple provider classes within the Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (16)
sickchill/oldbeard/providers/speedcd.py (1)
63-63: LGTM! Consider adding error details for debugging.The warning message improvement is good, adding the provider name makes logs more informative and helps with troubleshooting.
Consider capturing and logging the specific connection error for easier debugging:
- logger.warning(f"Unable to connect to provider {self.name}") + logger.warning(f"Unable to connect to provider {self.name}: Connection failed")sickchill/oldbeard/providers/hounddawgs.py (2)
55-55: Consider adding a defensive check for provider name.While the current implementation works, consider adding a defensive check to handle cases where
self.namemight not be set:- logger.warning(f"Unable to connect to provider {self.name}") + logger.warning(f"Unable to connect to provider {self.name or 'HoundDawgs'}")
Line range hint
31-65: Consider enhancing error handling granularity.The login method currently returns
Falsefor both connection failures and authentication failures. Consider introducing an enum or specific exceptions to help callers distinguish between different types of failures, which could enable more specific error handling or retry strategies.Example approach:
from enum import Enum class LoginError(Enum): CONNECTION_FAILED = "connection_failed" INVALID_CREDENTIALS = "invalid_credentials" COOKIE_ERROR = "cookie_error" def login(self) -> tuple[bool, Optional[LoginError]]: if any(dict_from_cookiejar(self.session.cookies).values()): return True, None # ... existing code ... if not response: return False, LoginError.CONNECTION_FAILED if re.search("Dit brugernavn eller kodeord er forkert.", response): return False, LoginError.INVALID_CREDENTIALS return True, Nonesickchill/oldbeard/providers/tntvillage.py (2)
108-108: LGTM! Consider enhancing error handling.The addition of the provider name to the warning message is a good improvement that aligns with the PR objective. However, consider enhancing the error handling to provide more specific failure reasons.
Consider capturing and logging the specific connection error:
- logger.warning(f"Unable to connect to provider {self.name}") + logger.warning(f"Unable to connect to provider {self.name}: Connection failed")Also consider adding error details when available:
try: response = self.get_url(self.urls["login"], post_data=login_params, returns="text") if not response: logger.warning(f"Unable to connect to provider {self.name}: No response received") return False except Exception as e: logger.warning(f"Unable to connect to provider {self.name}: {str(e)}") return False
Line range hint
92-116: Consider enhancing security and error handling in the login method.The login implementation could benefit from some security and robustness improvements:
Cookie handling:
- Consider validating cookie expiration time
- Add session timeout handling
Error handling:
- Add rate limiting detection
- Handle network timeouts explicitly
- Consider implementing retry logic for transient failures
Example implementation structure:
def login(self): # Check cookie validity including expiration if self._has_valid_cookies(): return True # Implement rate limiting if self._is_rate_limited(): logger.warning(f"{self.name}: Rate limit exceeded, waiting before retry") return False # Attempt login with retry for transient errors for attempt in range(MAX_RETRIES): try: response = self._attempt_login() if self._is_login_successful(response): return True elif self._is_credentials_error(response): logger.warning(f"{self.name}: Invalid username or password") return False except ConnectionError as e: logger.warning(f"{self.name}: Connection error: {str(e)}") if attempt == MAX_RETRIES - 1: return False time.sleep(RETRY_DELAY)sickchill/oldbeard/providers/morethantv.py (1)
Line range hint
71-72: Consider adding provider name to the invalid credentials warning.For consistency with the connection error message, consider including the provider name in the invalid credentials warning as well.
- logger.warning("Invalid username or password. Check your settings") + logger.warning(f"Invalid username or password for provider {self.name}. Check your settings")sickchill/oldbeard/providers/immortalseed.py (1)
Line range hint
63-73: Consider enhancing error handling with more actionable messages.While the current change is good, consider making the warning messages more actionable by including additional context like the provider's URL. This would help users troubleshoot connection issues more effectively.
Here's a suggested enhancement:
response = self.get_url(self.urls["login"], post_data=login_params, returns="text") if not response: - logger.warning(f"Unable to connect to provider {self.name}") + logger.warning(f"Unable to connect to provider {self.name} at {self.urls['login']}. Please check your internet connection and the provider's status.") return False if re.search("Username or password incorrect!", response): - logger.warning("Invalid username or password. Check your settings") + logger.warning(f"Invalid username or password for {self.name}. Please verify your credentials in the settings.") return Falsesickchill/oldbeard/providers/pretome.py (1)
Line range hint
143-143: Consider using f-string for traceback formatting.For consistency with the warning message changes, consider using an f-string here as well.
- logger.exception("Failed parsing provider. Traceback: {0}".format(traceback.format_exc())) + logger.exception(f"Failed parsing provider. Traceback: {traceback.format_exc()}")sickchill/oldbeard/providers/archetorrent.py (1)
51-51: Consider implementing a base login method in TorrentProviderThe login implementation and error handling are very similar across all provider classes. Consider moving the common login logic to the base
TorrentProviderclass and allow specific providers to override only the necessary parts.This would:
- Reduce code duplication
- Standardize error handling
- Make maintenance easier
Example implementation:
# In TorrentProvider class def login(self): if any(dict_from_cookiejar(self.session.cookies).values()): return True try: response = self._perform_login() if not response: logger.warning(f"Unable to connect to provider {self.name}") return False if not self._validate_login(response): logger.warning("Invalid username or password. Check your settings") return False return True except Exception as e: logger.error(f"Login failed for provider {self.name}: {str(e)}") return False # Provider classes would only need to implement: def _perform_login(self): # Provider-specific login logic pass def _validate_login(self, response): # Provider-specific validation passsickchill/oldbeard/providers/yggtorrent.py (1)
70-70: Consider differentiating the warning messages.While both warning messages now include the provider name, they could be more specific about which connection attempt failed (initial login vs. verification).
Consider updating the messages to be more specific:
- logger.warning(f"Unable to connect to provider {self.name}") + logger.warning(f"Unable to connect to provider {self.name} during initial login") - logger.warning(f"Unable to connect to provider {self.name}") + logger.warning(f"Unable to connect to provider {self.name} during login verification")Also applies to: 77-77
sickchill/oldbeard/providers/rarbg.py (1)
45-45: LGTM! Consider adding token context.The warning message now includes the provider name, improving log clarity. Since this is specifically a token retrieval failure, you might want to make the message even more specific.
Consider updating the message to include token context:
- logger.warning(f"Unable to connect to provider {self.name}") + logger.warning(f"Unable to connect to provider {self.name} for token retrieval")sickchill/oldbeard/providers/iptorrents.py (1)
59-59: Consider consolidating duplicate warning messagesThe same warning message appears twice in the login method. Consider consolidating the error handling to avoid duplicate code.
def login(self): cookie_dict = dict_from_cookiejar(self.session.cookies) if cookie_dict.get("uid") and cookie_dict.get("pass"): return True if self.cookies: success, status = self.add_cookies_from_ui() if not success: logger.info(status) return False login_params = {"username": self.username, "password": self.password, "login": "submit"} if self.custom_url: if self.invalid_url(self.custom_url): logger.warning(_("Invalid custom url: {0}").format(self.custom_url)) return False - # Get the index, redirects to log in - data = self.get_url(self.custom_url or self.url, returns="text") - if not data: - logger.warning(f"Unable to connect to provider {self.name}") - return False + try: + # Get the index, redirects to log in + data = self.get_url(self.custom_url or self.url, returns="text") + if not data: + raise ConnectionError() + with BS4Parser(data) as html: + action = html.find("form", {"action": re.compile(r".*login.*")}).get("action") + if not action: + logger.warning("Could not find the login form. Try adding cookies instead") + return False + response = self.get_url(urljoin(self.custom_url or self.url, action), post_data=login_params, returns="text", flaresolverr=True) + if not response: + raise ConnectionError() + except (ConnectionError, Exception): + logger.warning(f"Unable to connect to provider {self.name}") + return False - with BS4Parser(data) as html: - action = html.find("form", {"action": re.compile(r".*login.*")}).get("action") - if not action: - logger.warning("Could not find the login form. Try adding cookies instead") - return False - - response = self.get_url(urljoin(self.custom_url or self.url, action), post_data=login_params, returns="text", flaresolverr=True) - if not response: - logger.warning(f"Unable to connect to provider {self.name}") - return FalseAlso applies to: 70-70
sickchill/oldbeard/providers/tvchaosuk.py (3)
Line range hint
18-19: Consider using class constants for URLs.The URL strings could be moved to class-level constants for better maintainability and reusability.
class Provider(TorrentProvider): + BASE_URL = "https://www.tvchaosuk.com/" + LOGIN_PATH = "takelogin.php" + INDEX_PATH = "index.php" + SEARCH_PATH = "browse.php" + def __init__(self): super().__init__("TvChaosUK") - self.url = "https://www.tvchaosuk.com/" - self.urls = {"login": self.url + "takelogin.php", "index": self.url + "index.php", "search": self.url + "browse.php"} + self.url = self.BASE_URL + self.urls = { + "login": self.url + self.LOGIN_PATH, + "index": self.url + self.INDEX_PATH, + "search": self.url + self.SEARCH_PATH + }
Line range hint
28-31: Consider adding a more specific cookie validation.The current cookie validation uses a magic number (>= 4). Consider making this more explicit and documenting the expected cookies.
+ REQUIRED_COOKIES = {'uid', 'pass', 'session', 'authkey'} + def login(self): - if len(self.session.cookies) >= 4: + if all(cookie.name in self.session.cookies for cookie in self.REQUIRED_COOKIES): return True
Line range hint
57-142: Consider breaking down the search method for better maintainability.The search method is quite long and handles multiple responsibilities. Consider extracting the title parsing logic and torrent row parsing into separate methods.
Here's a suggested refactor for the title parsing logic:
+ def _parse_title(self, title, mode, search_keywords): + """Parse and clean up torrent title.""" + if mode == "Season": + title = re.sub(r"(.*)(?i)Series", r"\1Season", title) + + # Strip year from the end + title = re.sub(r"(.*)[. ]?\(\d{4}\)", r"\1", title) + title = re.sub(r"\s+", r" ", title) + + # Chop off tracker/channel prefix + if mode != "RSS" and search_keywords: + show_name_first_word = re.search(r"^[^ .]+", search_keywords).group() + if not title.startswith(show_name_first_word): + title = re.sub(r".*(" + show_name_first_word + ".*)", r"\1", title) + + return title + ".hdtv.x264"And for the torrent row parsing:
+ def _parse_torrent_row(self, torrent, labels, mode, search_keywords): + """Parse a single torrent row and return item dict.""" + if self.freeleech and not torrent.find("img", alt=re.compile("Free Torrent")): + return None + + title = torrent.find(class_="tooltip-content").div.get_text(strip=True) + download_url = torrent.find(title="Click to Download this Torrent!").parent["href"] + if not all([title, download_url]): + return None + + seeders = try_int(torrent.find(title="Seeders").get_text(strip=True)) + leechers = try_int(torrent.find(title="Leechers").get_text(strip=True)) + + if seeders < self.minseed or leechers < self.minleech: + if mode != "RSS": + logger.debug( + "Discarding torrent because it doesn't meet the" + " minimum seeders or leechers: {0} (S:{1} L:{2})".format(title, seeders, leechers) + ) + return None + + title = self._parse_title(title, mode, search_keywords) + torrent_size = torrent("td")[labels.index("Size")].get_text(strip=True) + size = convert_size(torrent_size) or -1 + + if mode != "RSS": + logger.debug( + _("Found result: {title} with {seeders} seeders and {leechers} leechers".format( + title=title, seeders=seeders, leechers=leechers + )) + ) + + return { + "title": title, + "link": download_url, + "size": size, + "seeders": seeders, + "leechers": leechers + }sickchill/oldbeard/providers/ncore.py (1)
Line range hint
41-48: Consider enhancing error handling consistency.The login method uses different logging levels for connection errors (warning) vs. authentication errors (warning). Consider adding debug-level details for connection failures to aid in troubleshooting.
response = self.get_url(self.urls["login"], post_data=login_params, returns="text") if not response: logger.warning(f"Unable to connect to provider {self.name}") + logger.debug(f"Failed to connect to {self.urls['login']}") return False if re.search("images/warning.png", response):
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
sickchill/oldbeard/providers/abnormal.py(1 hunks)sickchill/oldbeard/providers/alpharatio.py(1 hunks)sickchill/oldbeard/providers/archetorrent.py(1 hunks)sickchill/oldbeard/providers/bjshare.py(1 hunks)sickchill/oldbeard/providers/filelist.py(1 hunks)sickchill/oldbeard/providers/gimmepeers.py(1 hunks)sickchill/oldbeard/providers/hdspace.py(1 hunks)sickchill/oldbeard/providers/hdtorrents.py(1 hunks)sickchill/oldbeard/providers/hdtorrents_it.py(1 hunks)sickchill/oldbeard/providers/hounddawgs.py(1 hunks)sickchill/oldbeard/providers/immortalseed.py(1 hunks)sickchill/oldbeard/providers/iptorrents.py(2 hunks)sickchill/oldbeard/providers/morethantv.py(1 hunks)sickchill/oldbeard/providers/ncore.py(1 hunks)sickchill/oldbeard/providers/nebulance.py(1 hunks)sickchill/oldbeard/providers/pretome.py(1 hunks)sickchill/oldbeard/providers/rarbg.py(1 hunks)sickchill/oldbeard/providers/scc.py(1 hunks)sickchill/oldbeard/providers/scenetime.py(1 hunks)sickchill/oldbeard/providers/speedcd.py(1 hunks)sickchill/oldbeard/providers/tntvillage.py(1 hunks)sickchill/oldbeard/providers/torrentbytes.py(1 hunks)sickchill/oldbeard/providers/torrentday.py(1 hunks)sickchill/oldbeard/providers/torrentleech.py(1 hunks)sickchill/oldbeard/providers/tvchaosuk.py(1 hunks)sickchill/oldbeard/providers/yggtorrent.py(1 hunks)
🔇 Additional comments (19)
sickchill/oldbeard/providers/scc.py (1)
50-50: LGTM! Consider standardizing the warning message format across providers.
The addition of the provider name in the warning message improves log clarity and debugging capabilities. This change aligns well with the PR objective of enhancing warning logs.
For consistency across all providers, consider standardizing the message format to something like:
- logger.warning(f"Unable to connect to provider {self.name}")
+ logger.warning(f"Connection failed for provider: {self.name}")Let's verify the message format consistency across other providers:
sickchill/oldbeard/providers/hounddawgs.py (1)
55-55: LGTM! Enhanced warning message improves log clarity.
The addition of the provider name in the warning message aligns with the PR objectives and helps in better identifying which provider failed to connect.
sickchill/oldbeard/providers/torrentleech.py (1)
50-50: LGTM! Enhanced warning message clarity.
The addition of the provider name in the warning message improves log clarity and debugging capabilities.
sickchill/oldbeard/providers/abnormal.py (1)
52-52: LGTM! Enhanced warning message clarity.
The addition of the provider name in the warning message improves log clarity and debugging capabilities.
sickchill/oldbeard/providers/torrentbytes.py (2)
45-45: LGTM! Enhanced warning message clarity.
The addition of the provider name in the warning message improves log clarity and debugging capabilities.
Line range hint 45-52: Verify consistent warning message updates across all provider files.
Let's ensure that all provider files have been updated with the enhanced warning message.
sickchill/oldbeard/providers/scenetime.py (1)
50-50: LGTM! Consistent warning message enhancement.
The warning message update follows the same pattern as other providers, improving log clarity.
sickchill/oldbeard/providers/pretome.py (1)
54-54: LGTM! Consistent warning message enhancement.
The warning message update follows the same pattern as other providers, improving log clarity.
sickchill/oldbeard/providers/filelist.py (1)
47-47: LGTM! Warning message enhancement
The addition of the provider name {self.name} to the warning message improves log clarity and debugging capabilities.
sickchill/oldbeard/providers/gimmepeers.py (1)
59-59: LGTM! Warning message enhancement
The addition of the provider name {self.name} to the warning message improves log clarity and debugging capabilities.
sickchill/oldbeard/providers/torrentday.py (1)
64-64: LGTM! Warning message enhancement
The addition of the provider name {self.name} to the warning message improves log clarity and debugging capabilities.
sickchill/oldbeard/providers/hdspace.py (1)
57-57: LGTM! Warning message enhancement
The change improves log clarity by including the provider name in the warning message.
sickchill/oldbeard/providers/hdtorrents.py (1)
52-52: LGTM! Warning message enhancement
The change improves log clarity by including the provider name in the warning message.
sickchill/oldbeard/providers/archetorrent.py (1)
51-51: LGTM! Warning message enhancement
The change improves log clarity by including the provider name in the warning message.
sickchill/oldbeard/providers/nebulance.py (1)
55-55: LGTM! Enhanced warning message clarity.
The updated warning message now includes the provider name, making it easier to identify which provider failed to connect. The use of f-strings is also a good practice for string formatting.
sickchill/oldbeard/providers/hdtorrents_it.py (1)
52-52: LGTM: Improved warning message clarity
The warning message now includes the provider name using f-string formatting, which enhances log readability and debugging.
sickchill/oldbeard/providers/bjshare.py (1)
256-256: LGTM: Enhanced warning message clarity
The warning message now includes the provider name using f-string formatting, improving log readability and debugging capabilities.
sickchill/oldbeard/providers/tvchaosuk.py (1)
41-41: LGTM! Warning message enhancement improves log clarity.
The addition of the provider name in the warning message is a good improvement that helps identify which provider is experiencing connection issues.
sickchill/oldbeard/providers/ncore.py (1)
42-42: LGTM! The enhanced warning message improves log clarity.
The addition of the provider name in the warning message is a good improvement that will help with debugging connection issues.
Let's verify the consistent implementation across provider files and ensure self.name is properly initialized:
✅ Verification successful
Let's gather more information to verify the name initialization pattern, as the previous pattern might have been too specific.
Let's try one more verification with adjusted paths:
Based on the gathered information, I can now provide the final response:
The warning message enhancement is properly implemented and safe to use.
The verification shows:
self.nameis properly initialized in theNcoreProviderconstructor viasuper().__init__("ncore")- The warning message format is consistently implemented across multiple provider files
- The
self.nameattribute is widely used across the codebase in similar error handling scenarios
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify warning message consistency and name initialization across providers
# Check for consistent warning message format across provider files
echo "Checking warning message consistency:"
rg -l "Unable to connect to provider" sickchill/oldbeard/providers/
# Verify self.name initialization in provider constructors
echo -e "\nChecking name initialization in providers:"
ast-grep --pattern 'class Provider($_) {
def __init__($$$) {
super().__init__($name)
$$$
}
}'
Length of output: 1496
Script:
#!/bin/bash
# Check for name initialization patterns in provider files
echo "Checking name initialization in ncore.py:"
rg -A 5 "class NcoreProvider" sickchill/oldbeard/providers/ncore.py
echo -e "\nChecking base provider class definition:"
rg -A 10 "class GenericProvider" sickchill/oldbeard/providers/
# Check if name is set in the base provider class
echo -e "\nChecking name attribute usage in base provider:"
rg "self\.name" sickchill/oldbeard/providers/generic.py
Length of output: 573
Script:
#!/bin/bash
# Find the base provider class and ncore provider implementation
echo "Looking for provider base class:"
fd "provider" --type f | xargs rg -l "class.*Provider"
echo -e "\nChecking ncore provider implementation:"
cat sickchill/oldbeard/providers/ncore.py
echo -e "\nChecking for name attribute in all provider files:"
rg "self\.name" sickchill/oldbeard/providers/
Length of output: 11488
There was a problem hiding this comment.
Actionable comments posted: 27
🧹 Outside diff range and nitpick comments (57)
sickchill/oldbeard/providers/torrent_paradise.py (3)
41-41: Consider enhancing the log message with search context.While the internationalization is correctly implemented, consider including the search string in the log message to provide more context for debugging purposes.
- logger.debug(_("Data returned from provider does not contain any torrents")) + logger.debug(_("Data returned from provider does not contain any torrents for search: {search_string}").format(search_string=search_string))
Line range hint
11-12: Consider making the domain configurable.The hardcoded domain "torrent-paradise.ml" (.ml is Mali's TLD) might be unstable. Consider making it configurable through settings to facilitate easier maintenance if the domain changes.
Line range hint
71-72: Improve error handling in the search loop.The current error handling loses the original stack trace. Consider using
raisewithout arguments orraise from errorto preserve the traceback information.- raise error + raisesickchill/oldbeard/providers/hdbits.py (2)
72-72: LGTM: Proper internationalization of error message, with a minor suggestionThe error message has been correctly wrapped with
_()for internationalization support. However, consider making the message more specific to help with debugging.Consider this slightly more descriptive message:
- logger.exception(_("Resulting JSON from provider isn't correct, not parsing it")) + logger.exception(_("Received invalid JSON response from HDBits provider, unable to parse results"))
Line range hint
77-77: Address the "FIXME SORTING" commentThere's a TODO comment indicating that sorting needs to be implemented. This should be addressed to ensure proper ordering of search results.
Would you like me to help implement the sorting functionality or create a GitHub issue to track this task?
sickchill/oldbeard/providers/norbits.py (3)
38-41: Consider enhancing error feedbackThe method could be improved by including the actual error message from the JSON response and returning
Falseon authentication failure.@staticmethod def check_auth_from_data(parsed_json): """Check that we are authenticated.""" if "status" in parsed_json and "message" in parsed_json and parsed_json.get("status") == 3: - logger.warning(_("Invalid username or password. Check your settings")) + logger.warning(_("Authentication failed: {message}").format( + message=parsed_json.get("message", "Invalid username or password. Check your settings") + )) + return False return True
72-72: Simplify the string concatenationWhile the internationalization is correctly implemented, the string concatenation can be simplified.
- logger.exception(_("Resulting JSON from provider is not correct, " "not parsing it")) + logger.exception(_("Resulting JSON from provider is not correct, not parsing it"))
71-73: Enhance error debugging informationConsider adding more context about the incorrect JSON structure to help with debugging.
json_items = parsed_json.get("data", "") if not json_items: - logger.exception(_("Resulting JSON from provider is not correct, " "not parsing it")) + logger.exception(_("Incorrect JSON structure from provider. Expected 'data' field. Response: {response}").format( + response=parsed_json + ))sickchill/oldbeard/providers/shazbat.py (1)
37-37: Consider making the warning message more provider-specific.To improve troubleshooting, consider prefixing the warning message with the provider name.
- logger.warning(_("Invalid username or password. Check your settings")) + logger.warning(_("Shazbat.tv: Invalid username or password. Check your settings"))sickchill/oldbeard/providers/binsearch.py (1)
Line range hint
1-100: Consider architectural improvementsWhile reviewing the internationalization change, I noticed a few architectural points that might need attention:
- The provider is marked as
PROVIDER_BROKEN(line 20) but still implements full functionality- The newsgroups in
update_cache()are hardcoded, which might be better as configurationConsider:
- Documenting why the provider is marked as broken or removing the flag if it's working
- Moving the newsgroups list to a configuration file for better maintainability
sickchill/oldbeard/providers/demonoid.py (3)
53-59: Consider using consistent string formattingWhile the internationalization is correct, the string formatting could be more consistent. Consider using the same formatting style as other messages in the codebase.
- logger.debug(_("Search string: {0}".format(search_string))) + logger.debug(_("Search string: {search_string}").format(search_string=search_string))
113-113: Consider using consistent string formattingSimilar to the earlier suggestion, consider updating the string formatting style for consistency.
- logger.debug(_("Found result: {0} with {1} seeders and {2} leechers".format(title, seeders, leechers))) + logger.debug(_("Found result: {title} with {seeders} seeders and {leechers} leechers").format( + title=title, seeders=seeders, leechers=leechers))
Line range hint
1-144: Consider establishing string formatting guidelinesWhile all logging messages have been properly internationalized, there are inconsistencies in string formatting styles throughout the file. Consider establishing project-wide guidelines for string formatting in internationalized messages to maintain consistency across the codebase.
Options to consider:
- Named placeholders:
_("Message: {name}").format(name=value)- Positional placeholders:
_("Message: {0}").format(value)- f-strings (if compatible with your internationalization setup)
Having consistent formatting will make the code more maintainable and easier to review.
sickchill/oldbeard/providers/rsstorrent.py (1)
103-103: Consider using f-strings for better readability.While the internationalization is correct, the string formatting could be modernized.
- logger.exception(_("Skipping RSS Torrent provider string: {0}, incorrect format".format(config))) + logger.exception(_(f"Skipping RSS Torrent provider string: {config}, incorrect format"))sickchill/oldbeard/providers/eztv.py (1)
Line range hint
42-78: Overall implementation feedbackWhile the internationalization changes are a step in the right direction, there's a consistent pattern of placing string formatting inside the translation function. This should be adjusted throughout the file for better maintainability and translation support.
Consider creating a follow-up PR to improve error handling and API response validation in the search method.
sickchill/oldbeard/providers/newznab.py (2)
238-238: LGTM with a minor suggestion for string formattingThe exception message has been correctly wrapped with
_()for internationalization. However, consider using f-strings or the newer.format()style for better readability:-logger.exception(_("Skipping Newznab provider string: '{0}', incorrect format".format(config))) +logger.exception(_("Skipping Newznab provider string: '{}', incorrect format").format(config)) # or using f-string (preferred) +logger.exception(_("Skipping Newznab provider string: '{config}', incorrect format"))
Missing internationalization in log messages
The following log messages need to be wrapped with the
_()function for internationalization:
- Line ~130:
logger.warning(error_string)whereerror_string = "Error getting caps xml for [{0}]".format(self.name)- Line ~140:
logger.debug(error_string)whereerror_string = "Error parsing caps xml for [{0}]".format(self.name)- Line ~250:
logger.info(err_desc)whereerr_descis from external data🔗 Analysis chain
Line range hint
192-321: Verify internationalization coverageLet's verify that all log messages in this file are properly internationalized.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining non-internationalized log messages # Search for logger calls without _() wrapper rg -U 'logger\.(debug|info|warning|error|exception)\([^_]' sickchill/oldbeard/providers/newznab.py # Search for string literals in logger calls rg -U 'logger\.(debug|info|warning|error|exception)\(_\(".*?"\)\)' sickchill/oldbeard/providers/newznab.pyLength of output: 471
Script:
#!/bin/bash # Let's get more context around the non-internationalized log messages # Get context for logger calls without _() wrapper rg -U -B 2 -A 2 'logger\.(warning|debug|info)\([^_]' sickchill/oldbeard/providers/newznab.pyLength of output: 626
sickchill/oldbeard/providers/omgwtfnzbs.py (2)
39-41: Consider using new-style string formattingWhile the internationalization is correct, consider using f-strings or str.format() with named parameters for better readability and maintainability:
- logger.debug(_("Unknown error: {0}".format(description_text))) + logger.debug(_("Unknown error: {error}").format(error=description_text))
Line range hint
74-74: Replace string concatenation with format stringFor consistency with other logging messages and better internationalization support, avoid string concatenation:
- logger.debug(_("Found result: ") + f'{item.get("release")}') + logger.debug(_("Found result: {name}").format(name=item.get("release")))sickchill/oldbeard/providers/tokyotoshokan.py (1)
88-88: Consider moving string formatting outside the _() call.While the internationalization is correct, it's better practice to move the
.format()call outside the_()function:-logger.debug(_("Found result: {0} with {1} seeders and {2} leechers".format(title, seeders, leechers))) +logger.debug(_("Found result: {0} with {1} seeders and {2} leechers").format(title, seeders, leechers))This allows translators to work with the complete string template and prevents potential issues with different language word orders.
sickchill/oldbeard/providers/newpct.py (2)
135-135: LGTM: Properly internationalized info messageThe info message has been correctly wrapped with the
_()function. While the current string formatting works, consider using f-strings in a future cleanup PR for better readability:logger.info(_("Downloading a result from {url}").format(url=url)) # or even better logger.info(_("Downloading a result from %(url)s"), {"url": url})
135-146: Consider standardizing string formatting approach across log messagesWhile all the internationalization changes are correct, the codebase uses different string formatting approaches. Consider standardizing on a single approach in a future cleanup PR:
- Using named parameters:
_("Message %(param)s") % {"param": value}- Using str.format:
_("Message {param}").format(param=value)- Using f-strings:
_(f"Message {param}")(though this approach is discouraged for i18n)This would improve maintainability and make the code more consistent.
sickchill/oldbeard/providers/bitcannon.py (3)
44-44: Improve string formatting for better translation supportThe current implementation nests the
.format()call inside the_()function, which could cause issues with translation ordering in some languages.Consider this alternative approach:
-logger.debug(_("Search string: {0}".format(search_string))) +logger.debug(_("Search string: {search_string}").format(search_string=search_string))This way, translators can reorder the placeholder if needed for their language.
82-82: Improve string formatting for complex translationsThe current implementation with multiple placeholders nested inside
_()could make it difficult for translators to handle different word orders in various languages.Consider this alternative approach:
-logger.debug(_("Found result: {0} with {1} seeders and {2} leechers".format(title, seeders, leechers))) +logger.debug(_("Found result: {title} with {seeders} seeders and {leechers} leechers").format( + title=title, + seeders=seeders, + leechers=leechers +))This makes the translation more flexible and clearer for translators.
Consider standardizing string formatting to use f-strings or str.format() with named parameters
The verification confirms that inconsistent string formatting patterns are widespread across provider files. Most logging statements use positional placeholders with
.format(), which can be harder to maintain and translate. For better maintainability and translation flexibility:
- Use f-strings for simple cases:
logger.debug(_(f"Found {title} with {seeders} seeders"))- Use named parameters for complex cases:
logger.debug(_("Found result: {title} with {seeders} seeders").format(title=title, seeders=seeders))Examples of current inconsistencies found:
"Found result: {0} with {1} seeders and {2} leechers".format(title, seeders, leechers)"{url} did not return any data".format(url=self.url)"Found {} results".format(len(html))🔗 Analysis chain
Line range hint
26-97: Consider standardizing the string formatting approach across all messagesWhile the internationalization changes are good, consider adopting a consistent approach to string formatting across all messages. Using named placeholders consistently would make the code more maintainable and translations more flexible.
Let's check if this pattern exists in other provider files:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for similar string formatting patterns in other provider files rg -U 'logger\.(debug|warning|info|error)\(_\([^)]+\.format\(' sickchill/oldbeard/providers/Length of output: 12621
sickchill/oldbeard/providers/nyaa.py (2)
57-57: LGTM! Internationalization properly implemented.The change correctly wraps the log message with the translation function.
Consider also internationalizing other debug messages in this file for consistency, particularly the error message at line 76:
- logger.debug(f"Error parsing results: {error}") + logger.debug(_("Error parsing results: {error}").format(error=error))
Line range hint
71-73: Maintain consistent internationalization for all user-facing messages.For consistency with the other changes, consider internationalizing the seeders/leechers message:
- logger.debug( - f"Discarding torrent because it doesn't meet the minimum seeders or leechers: {result['title']} (S:{result['seeders']} L:{result['leechers']})" - ) + logger.debug( + _("Discarding torrent because it doesn't meet the minimum seeders or leechers: {title} (S:{seeders} L:{leechers})").format( + title=result['title'], + seeders=result['seeders'], + leechers=result['leechers'] + ) + )sickchill/oldbeard/providers/skytorrents.py (1)
80-80: Improve string formatting for better translation supportThe current implementation uses positional parameters which can make translation difficult as translators need to maintain the exact order.
Consider using named parameters for better translation support:
- logger.debug(_("Found result: {0} with {1} seeders and {2} leechers".format(title, seeders, leechers))) + logger.debug(_("Found result: {title} with {seeders} seeders and {leechers} leechers").format( + title=title, seeders=seeders, leechers=leechers))sickchill/oldbeard/providers/thepiratebay.py (1)
153-153: Consider improving the message format for better translationWhile the internationalization is correct, the message structure could be improved for clarity and translation.
Consider this alternative:
- logger.debug(_("Found result: {0} but that doesn't seem like a trusted result so I'm ignoring it".format(title))) + logger.debug(_("Ignoring untrusted result: {0}").format(title))sickchill/oldbeard/providers/danishbits.py (5)
54-54: Consider improving string formatting for better translation.While the internationalization is correct, the current nested format approach could be problematic for translators. Consider separating the translation and formatting:
-logger.debug(_("Search string: {0}".format(search_string))) +logger.debug(_("Search string: {0}").format(search_string))
86-86: Improve string formatting for complex translated message.Similar to the earlier case, the nested format approach should be avoided for better translation support, especially with multiple parameters:
-logger.debug(_("Found result: {0} with {1} seeders and {2} leechers".format(title, seeders, leechers))) +logger.debug(_("Found result: {0} with {1} seeders and {2} leechers").format(title, seeders, leechers))
Line range hint
89-90: Consider internationalizing the warning message.For consistency, the warning message should also be internationalized:
- logger.warning(result["error"]) + logger.warning(_("Provider error: {0}").format(result["error"]))
Line range hint
74-79: Internationalize the info log message.For consistency with other log messages, this info message should also be internationalized:
- logger.info( - "Discarded {0} because with {1}/{2} seeders/leechers does not meet the requirement of {3}/{4} seeders/leechers".format( - title, seeders, leechers, self.minseed, self.minleech - ) - ) + logger.info( + _("Discarded {0} because with {1}/{2} seeders/leechers does not meet the requirement of {3}/{4} seeders/leechers").format( + title, seeders, leechers, self.minseed, self.minleech + ) + )
Non-internationalized log messages found across provider files
Several provider files contain non-internationalized log messages that should be wrapped with
_(). Here are some examples:
cpasbien.py: Direct string usage in logger calls likelogger.debug(f"Found result: {title}...")torrent9.py: Similar f-string usage without internationalizationnewznab.py: Direct string usage inlogger.warning()andlogger.info()kat.py: Multiple instances of non-internationalized loggingdanishbits.py: Non-internationalizedlogger.info()andlogger.warning()callsAdditionally, some files have potentially problematic nested format calls within
_()that should be refactored to use f-strings, for example in:
yggtorrent.py:_("Changing base url from {} to {}".format(self.url, new_url))torrentproject.py:_("Found result: {0} with {1} seeders and {2} leechers".format(title, seeders, leechers))🔗 Analysis chain
Line range hint
1-1: Verify consistent internationalization across all providers.Let's ensure all provider files have been updated with consistent internationalization:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for non-internationalized log messages in provider files # Look for logger calls without _() wrapper echo "Checking for non-internationalized log messages in provider files..." rg -U 'logger\.(debug|info|warning|error)\([^_]' sickchill/oldbeard/providers/ echo "Checking for potentially problematic nested format calls..." rg -U 'logger\.(debug|info|warning|error)\(_\([^)]+\.format\(' sickchill/oldbeard/providers/Length of output: 22320
sickchill/oldbeard/providers/horriblesubs.py (2)
Line range hint
169-169: Fix inconsistent string formatting approachThe debug message uses a mix of translation concatenation and f-strings, which should be converted to use the same consistent approach as other messages.
Apply this change:
-logger.debug(_("Found result: ") + f"{release_title}") +logger.debug(_("Found result: {0}").format(release_title))
Line range hint
1-169: Consider standardizing logging approach across the providerWhile adding internationalization support is good, the implementation uses different string formatting approaches throughout the file. Consider standardizing on a single approach for all log messages to improve maintainability.
Options to consider:
- Use
.format()consistently (current approach)- Switch to f-strings throughout (more modern)
- Use
%formatting (if maintaining compatibility with older code)Whatever approach is chosen, it should be applied consistently across all logging calls in the provider.
sickchill/oldbeard/providers/magnetdl.py (3)
49-49: Consider splitting the debug message for clarityThe debug message combines both the error state and a suggestion. Consider splitting it into two separate messages for better clarity and easier translation.
- logger.debug(_("URL did not return results/data, if the results are on the site maybe try a custom url, or a different one")) + logger.debug(_("URL did not return results/data")) + logger.debug(_("If the results exist on the site, try using a custom URL"))
Line range hint
84-87: Internationalize the debug message about discarding torrentsFor consistency, this debug message should also be wrapped with the
_()function.- logger.debug( - "Discarding torrent because it doesn't meet the" - " minimum seeders or leechers: {0} (S:{1} L:{2})".format(title, seeders, leechers) - ) + logger.debug( + _("Discarding torrent because it doesn't meet the minimum seeders or leechers: {title} (S:{seeders} L:{leechers})").format( + title=title, seeders=seeders, leechers=leechers + ) + )
Inconsistent internationalization found in provider log messages
Several providers have non-internationalized log messages that should be wrapped with
_(). Key issues:
- Using f-strings directly with logger calls:
logger.info(f"Number of torrents found on {self.name} = {len(data)}") logger.debug(f"Failed parsing provider. Traceback: {error}")
- Direct string usage without internationalization:
logger.debug(title) logger.info(status) logger.info(error)
- Inconsistent format string usage:
logger.debug(f"Found result: {title} with {seeders} seeders") # vs correct internationalized version logger.debug(_("Found result: {0} with {1} seeders").format(title, seeders))Affected files:
- sickchill/oldbeard/providers/torrent_paradise.py
- sickchill/oldbeard/providers/magnetdl.py
- sickchill/oldbeard/providers/nyaa.py
- sickchill/oldbeard/providers/torrent911.py
- sickchill/oldbeard/providers/cpasbien.py
- And several others
🔗 Analysis chain
Line range hint
1-100: Verify consistent internationalization across all providersLet's verify that similar log messages in other provider files are also properly internationalized.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for non-internationalized log messages in provider files rg -U 'logger\.(debug|info|warning|error)\([^_].*?\)' 'sickchill/oldbeard/providers/' # Search for potentially problematic string formatting in internationalized messages rg -U 'logger\.(debug|info|warning|error)\(_\([^)]+\.format\(' 'sickchill/oldbeard/providers/'Length of output: 26682
sickchill/oldbeard/providers/hd4free.py (3)
28-28: Consider using named placeholders for better i18n supportWhile the internationalization wrapper is correctly applied, using
.format()with positional arguments can cause issues with translation ordering in some languages. Consider using named placeholders for better flexibility in translations.- logger.warning(_("Your authentication credentials for {0} are missing, check your config.".format(self.name))) + logger.warning(_("Your authentication credentials for %(name)s are missing, check your config.") % {"name": self.name})
56-60: Consider consolidating duplicate log messagesThe same debug message appears twice in close proximity. Consider refactoring to avoid duplication.
try: jdata = self.get_url(self.urls["search"], params=search_params, returns="json") except ValueError: - logger.debug(_("No data returned from provider")) continue if not jdata: - logger.debug(_("No data returned from provider")) continueExtract the message to a single location:
NO_DATA_MESSAGE = _("No data returned from provider") # Then use logger.debug(NO_DATA_MESSAGE) in both places
Line range hint
1-109: Consider implementing a centralized message catalogWhile the changes correctly implement i18n support, consider creating a centralized message catalog for all provider-related messages. This would:
- Ensure consistency across all providers
- Make it easier to maintain and update translations
- Reduce duplicate messages across different providers
- Allow for better message categorization and management
Consider creating a new module like
sickchill/oldbeard/providers/messages.pyto store all provider-related messages as constants.sickchill/oldbeard/providers/btn.py (1)
Line range hint
106-129: Consider improving error handling architectureThe
_api_callmethod currently handles multiple error conditions but uses generic exception handling. Consider:
- Creating custom exceptions for specific API errors
- Using a more structured approach to error handling
This would make it easier for calling code to handle specific error cases.
Example implementation:
class BTNAPIError(Exception): """Base exception for BTN API errors""" pass class InvalidAPIKeyError(BTNAPIError): """Raised when the API key is invalid""" pass class CallLimitExceededError(BTNAPIError): """Raised when the API call limit is exceeded""" pass def _api_call(self, params=None, results_per_page=1000, offset=0) -> dict: try: data = jsonrpclib.Server(self.urls["base_url"]).getTorrents( self.api_key, params or {}, int(results_per_page), int(offset) ) time.sleep(cpu_presets[settings.CPU_PRESET]) return data except jsonrpclib.jsonrpc.ProtocolError as error: if error == (-32001, "Invalid API Key"): logger.warning(_("The API key provided was rejected because it is invalid")) raise InvalidAPIKeyError("Invalid API Key") elif error == (-32002, "Call Limit Exceeded"): logger.warning(_("You have exceeded the limit of 150 calls per hour")) raise CallLimitExceededError("Call limit exceeded") raise BTNAPIError(f"JSON-RPC protocol error: {error}") except socket.timeout: logger.warning(_("Timeout while accessing provider")) raise BTNAPIError("Connection timeout") except socket.error as error: logger.warning(_("Socket error while accessing provider: {0}").format(error)) raise BTNAPIError(f"Socket error: {error}") except Exception as error: logger.warning(_("Unknown error while accessing provider: {0}").format(error)) raise BTNAPIError(f"Unknown error: {error}")sickchill/oldbeard/providers/elitetorrent.py (2)
54-54: Consider using f-strings for better readability.While the internationalization is correct, the string formatting could be modernized.
- logger.debug(_("Search string: {0}".format(search_string))) + logger.debug(_("Search string: {search_string}"))
Line range hint
49-133: Consider standardizing logging message format across providers.While the internationalization changes are correct, consider creating a standardized logging format for all providers. This could include:
- Consistent message structure
- Standard way of formatting variables
- Common set of log levels for similar events
- Separation of technical details from translatable messages
This would improve maintainability and provide a more consistent user experience.
sickchill/oldbeard/providers/limetorrents.py (2)
49-49: Consider enhancing the error message with more detailsWhile the internationalization is correct, consider making the message more informative by including details about what was actually received.
- logger.info(_("Expected xml but got something else, is your mirror failing?")) + logger.info(_("Expected XML but received {content_type}. Check if mirror is working.").format( + content_type=data[:30] + '...' if len(data) > 30 else data + ))
Line range hint
1-110: Consider implementing structured loggingWhile internationalizing log messages is good, consider implementing structured logging to improve log parsing and analysis. This would involve:
- Adding consistent log fields across all providers
- Including machine-readable error codes
- Separating user-facing messages from technical details
Example fields to consider: provider_name, action_type (search/parse/download), error_code, etc.
sickchill/oldbeard/providers/kat.py (1)
Line range hint
75-152: Consider standardizing string formatting patternsWhile all log messages have been properly internationalized, there are inconsistencies in the string formatting patterns used throughout the file. Some messages use positional formatting
{0}, others use named formatting{url}, and some use direct string concatenation.Consider standardizing on named formatting parameters throughout the file for better maintainability and clarity.
🧰 Tools
🪛 Ruff (0.8.2)
87-87: Local variable
labelsis assigned to but never usedRemove assignment to unused variable
labels(F841)
sickchill/oldbeard/providers/xthor.py (4)
25-25: Consider using f-strings for better readabilityWhile the internationalization wrapper is correctly applied, consider using f-strings for more readable string formatting:
- logger.warning(_("Your authentication credentials for {0} are missing, check your config.".format(self.name))) + logger.warning(_("Your authentication credentials for {name} are missing, check your config.").format(name=self.name))
Line range hint
29-31: Fix critical authentication check bugThe current check
if not self._check_auth:is comparing the method reference instead of calling the method. This will always evaluate toFalseas the method exists, effectively bypassing the authentication check.- if not self._check_auth: + if not self._check_auth():
61-63: Enhance account status warning with more detailsThe warning message could be more helpful by including the actual ratio information, and the error handling could be improved:
- Add ratio information to the warning message
- Consider logging the full account status details at debug level before returning
+ logger.debug(_("Account Status: {status}").format(status=jdata.get("user", {}))) logger.warning(_("Sorry, your account is not allowed to download, check your ratio")) return results
Line range hint
29-67: Consider implementing a more robust error handling strategyThe current error handling approach in the search method has several areas that could benefit from a more structured approach:
- Authentication failures
- API errors
- Account status issues
- Empty results handling
Consider implementing a custom exception hierarchy for different error types and handling them consistently throughout the provider implementations.
Would you like me to propose a detailed error handling strategy that could be applied across all providers?
sickchill/oldbeard/providers/ilcorsaronero.py (5)
91-91: Consider using f-strings for better readabilityWhile the internationalization wrapper is correct, consider using f-strings for better readability and performance.
-logger.debug(_("Episode quality: {0}".format(file_quality))) +logger.debug(_(f"Episode quality: {file_quality}"))
194-194: Consider using f-strings for better readabilityThe format() method could be replaced with an f-string for better readability while maintaining internationalization.
-logger.debug(_("Processing page {0} of results".format(page))) +logger.debug(_(f"Processing page {page} of results"))
249-249: Consider using f-strings for better readabilityThe format() method could be replaced with f-strings while maintaining internationalization.
-logger.debug(_("Torrent is subtitled, skipping: {0}".format(title))) -logger.debug(_("Torrent isn't english audio/subtitled, skipping: {0}".format(title))) +logger.debug(_(f"Torrent is subtitled, skipping: {title}")) +logger.debug(_(f"Torrent isn't english audio/subtitled, skipping: {title}"))Also applies to: 253-253
Line range hint
277-290: Consider improving readability of complex log messagesWhile the internationalization is correct, the nested formatting could be simplified for better readability.
- logger.debug( - _("Discarding torrent because it doesn't meet the minimum seeders or leechers: {0} (S:{1} L:{2})").format( - title, seeders, leechers - ) - ) + logger.debug( + _("Discarding torrent because it doesn't meet the minimum seeders or leechers: " + "{title} (S:{seeders} L:{leechers})").format( + title=title, seeders=seeders, leechers=leechers + ) + ) - logger.debug( - _( - "Found result: {title} with {seeders} seeders and {leechers} leechers".format( - title=title, seeders=seeders, leechers=leechers - ) - ) - ) + logger.debug( + _("Found result: {title} with {seeders} seeders and {leechers} leechers").format( + title=title, seeders=seeders, leechers=leechers + ) + )
Line range hint
1-290: Overall internationalization implementation is good but could be more consistentThe changes successfully wrap all logging messages with _() for internationalization, addressing the warning-log issue. However, there are inconsistencies in string formatting approaches (concatenation vs. format() vs. f-strings) that could be standardized for better maintainability.
Consider:
- Standardizing on f-strings for simple string formatting
- Using named format parameters for complex messages
- Creating helper methods for frequently repeated log messages
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (56)
sickchill/helper/common.py(1 hunks)sickchill/oldbeard/providers/abnormal.py(3 hunks)sickchill/oldbeard/providers/alpharatio.py(4 hunks)sickchill/oldbeard/providers/archetorrent.py(3 hunks)sickchill/oldbeard/providers/binsearch.py(1 hunks)sickchill/oldbeard/providers/bitcannon.py(4 hunks)sickchill/oldbeard/providers/bjshare.py(4 hunks)sickchill/oldbeard/providers/btn.py(4 hunks)sickchill/oldbeard/providers/danishbits.py(2 hunks)sickchill/oldbeard/providers/demonoid.py(4 hunks)sickchill/oldbeard/providers/elitetorrent.py(3 hunks)sickchill/oldbeard/providers/eztv.py(2 hunks)sickchill/oldbeard/providers/filelist.py(3 hunks)sickchill/oldbeard/providers/gimmepeers.py(6 hunks)sickchill/oldbeard/providers/hd4free.py(4 hunks)sickchill/oldbeard/providers/hdbits.py(2 hunks)sickchill/oldbeard/providers/hdspace.py(5 hunks)sickchill/oldbeard/providers/hdtorrents.py(6 hunks)sickchill/oldbeard/providers/hdtorrents_it.py(6 hunks)sickchill/oldbeard/providers/horriblesubs.py(2 hunks)sickchill/oldbeard/providers/hounddawgs.py(5 hunks)sickchill/oldbeard/providers/ilcorsaronero.py(6 hunks)sickchill/oldbeard/providers/immortalseed.py(5 hunks)sickchill/oldbeard/providers/iptorrents.py(3 hunks)sickchill/oldbeard/providers/kat.py(4 hunks)sickchill/oldbeard/providers/limetorrents.py(2 hunks)sickchill/oldbeard/providers/magnetdl.py(3 hunks)sickchill/oldbeard/providers/morethantv.py(4 hunks)sickchill/oldbeard/providers/ncore.py(4 hunks)sickchill/oldbeard/providers/nebulance.py(3 hunks)sickchill/oldbeard/providers/newpct.py(3 hunks)sickchill/oldbeard/providers/newznab.py(4 hunks)sickchill/oldbeard/providers/norbits.py(2 hunks)sickchill/oldbeard/providers/nyaa.py(1 hunks)sickchill/oldbeard/providers/omgwtfnzbs.py(2 hunks)sickchill/oldbeard/providers/pretome.py(4 hunks)sickchill/oldbeard/providers/rarbg.py(4 hunks)sickchill/oldbeard/providers/rsstorrent.py(1 hunks)sickchill/oldbeard/providers/scc.py(3 hunks)sickchill/oldbeard/providers/scenetime.py(3 hunks)sickchill/oldbeard/providers/shazbat.py(1 hunks)sickchill/oldbeard/providers/skytorrents.py(2 hunks)sickchill/oldbeard/providers/speedcd.py(3 hunks)sickchill/oldbeard/providers/thepiratebay.py(4 hunks)sickchill/oldbeard/providers/tntvillage.py(8 hunks)sickchill/oldbeard/providers/tokyotoshokan.py(2 hunks)sickchill/oldbeard/providers/torrent_paradise.py(1 hunks)sickchill/oldbeard/providers/torrentbytes.py(4 hunks)sickchill/oldbeard/providers/torrentday.py(2 hunks)sickchill/oldbeard/providers/torrentleech.py(4 hunks)sickchill/oldbeard/providers/torrentproject.py(3 hunks)sickchill/oldbeard/providers/torrentz.py(2 hunks)sickchill/oldbeard/providers/tvchaosuk.py(2 hunks)sickchill/oldbeard/providers/xthor.py(3 hunks)sickchill/oldbeard/providers/yggtorrent.py(6 hunks)sickchill/providers/torrent/FrenchProvider.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- sickchill/helper/common.py
🚧 Files skipped from review as they are similar to previous changes (26)
- sickchill/oldbeard/providers/bjshare.py
- sickchill/oldbeard/providers/yggtorrent.py
- sickchill/oldbeard/providers/abnormal.py
- sickchill/oldbeard/providers/torrentleech.py
- sickchill/oldbeard/providers/filelist.py
- sickchill/oldbeard/providers/tntvillage.py
- sickchill/oldbeard/providers/iptorrents.py
- sickchill/oldbeard/providers/hdtorrents_it.py
- sickchill/oldbeard/providers/hdspace.py
- sickchill/oldbeard/providers/rarbg.py
- sickchill/oldbeard/providers/torrentday.py
- sickchill/oldbeard/providers/hdtorrents.py
- sickchill/oldbeard/providers/pretome.py
- sickchill/oldbeard/providers/speedcd.py
- sickchill/oldbeard/providers/archetorrent.py
- sickchill/oldbeard/providers/morethantv.py
- sickchill/oldbeard/providers/torrentbytes.py
- sickchill/oldbeard/providers/scenetime.py
- sickchill/oldbeard/providers/tvchaosuk.py
- sickchill/oldbeard/providers/scc.py
- sickchill/oldbeard/providers/gimmepeers.py
- sickchill/oldbeard/providers/alpharatio.py
- sickchill/oldbeard/providers/ncore.py
- sickchill/oldbeard/providers/hounddawgs.py
- sickchill/oldbeard/providers/immortalseed.py
- sickchill/oldbeard/providers/nebulance.py
🧰 Additional context used
🪛 Ruff (0.8.2)
sickchill/oldbeard/providers/kat.py
87-87: Local variable labels is assigned to but never used
Remove assignment to unused variable labels
(F841)
120-120: Undefined name size
(F821)
🔇 Additional comments (52)
sickchill/oldbeard/providers/torrent_paradise.py (1)
37-37: LGTM! Log message properly internationalized.
The debug message has been correctly wrapped with the _() function for internationalization support.
sickchill/providers/torrent/FrenchProvider.py (1)
82-82: LGTM: Internationalization support added.
The warning message has been properly wrapped with _() for internationalization support, aligning with the broader effort to improve logging across provider classes.
sickchill/oldbeard/providers/hdbits.py (1)
39-39: LGTM: Proper internationalization of authentication warning message
The warning message has been correctly wrapped with _() for internationalization support, consistent with the project's localization standards.
sickchill/oldbeard/providers/norbits.py (1)
39-39: LGTM: Internationalization properly implemented
The warning message has been correctly wrapped with _() for internationalization support.
sickchill/oldbeard/providers/shazbat.py (2)
37-37: LGTM! Internationalization properly implemented.
The warning message has been correctly wrapped with _() for internationalization support.
37-37: Verify consistency of auth warning messages across providers.
Let's check if other providers follow a similar pattern and could benefit from consistent warning messages.
✅ Verification successful
Let me check the actual warning messages to ensure consistency.
Warning message is consistent across providers
The warning message "Invalid username or password. Check your settings" is already standardized across all providers, with only one minor variation in pretome.py which includes an additional PIN message. The implementation in shazbat.py follows the same consistent pattern used throughout the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for similar authentication warning patterns in other providers
# to ensure consistency in error messaging.
# Search for warning messages in provider files
rg -l "logger.warning.*password" "sickchill/oldbeard/providers/"
# Search for check_auth_from_data implementations
ast-grep --pattern 'def check_auth_from_data($$$)'
Length of output: 1246
Script:
#!/bin/bash
# Get the actual warning messages related to password/authentication
rg "logger.warning.*password" "sickchill/oldbeard/providers/" -A 1
Length of output: 6120
sickchill/oldbeard/providers/binsearch.py (1)
85-85: Verify internationalization of other log messages
While this change correctly internationalizes one log message, we should ensure consistency across all logging statements in this file.
Let's check for other logging statements that might need internationalization:
✅ Verification successful
Internationalization is consistent across providers for this log message
The verification shows that the log message "No data returned from provider" is already properly internationalized using the _() function across all provider files where it appears. In the specific file binsearch.py, this is the only logging statement present, and it's correctly internationalized.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other logging statements in this file that might need internationalization
rg --type py "logger\.(debug|info|warning|error|exception|log)" sickchill/oldbeard/providers/binsearch.py
# Check how other provider files handle similar logging
rg --type py "No data returned from provider" sickchill/oldbeard/providers/
Length of output: 3512
sickchill/oldbeard/providers/demonoid.py (4)
48-48: LGTM: Clear informative message with proper internationalization
The warning message about RSS search limitation is well-formatted and properly internationalized.
81-81: LGTM: Clear debug message for discarded items
The debug message clearly explains why the torrent is being discarded and is properly internationalized.
Line range hint 90-95: LGTM: Comprehensive debug message for filtered results
The debug message provides clear information about why torrents are filtered based on seeder/leecher criteria. Good use of internationalization and string formatting.
132-132: LGTM: Clear debug message
The debug message is concise and properly internationalized.
sickchill/oldbeard/providers/rsstorrent.py (1)
103-103: LGTM! Internationalization properly implemented.
The log message is now correctly wrapped with _() for translation support, which aligns with the PR objective to improve warning logs.
sickchill/oldbeard/providers/eztv.py (1)
49-49: LGTM!
The translation wrapper is correctly applied to the static message.
sickchill/oldbeard/providers/newznab.py (2)
192-192: LGTM: Warning message properly internationalized
The warning message for invalid API key has been correctly wrapped with the _() function for internationalization support.
309-309: LGTM: Debug messages properly internationalized
Both debug messages have been correctly wrapped with the _() function for internationalization support:
- Search mode logging
- No data returned logging
Also applies to: 321-321
sickchill/oldbeard/providers/omgwtfnzbs.py (2)
23-23: LGTM: Properly internationalized warning message
The warning message for invalid API key has been correctly wrapped with _() for internationalization support.
Line range hint 65-74: Verify logging consistency across providers
The logging pattern implementation looks good, but we should verify consistency across other provider implementations.
✅ Verification successful
Based on the comprehensive results from the search, I can now generate the final response:
Logging pattern is consistent across providers
The verification shows that all providers consistently use the _() function for internationalization with logger calls. The pattern matches the implementation in the reviewed code:
- All debug/warning messages use
_()wrapper for translation - Format strings consistently use either
.format()or f-strings - Common patterns like
_("Search Mode: {mode}")and_("Search String: {search_string}")are used across providers
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check logging patterns in other provider files
# Expected: Similar _() usage for debug/warning messages
# Check for inconsistent logging patterns in provider files
rg -U 'logger\.(debug|warning)(?!.*_\().*\n' 'sickchill/oldbeard/providers/'
# Check format string usage patterns
rg 'logger\.(debug|warning)\(_\([^)]+\)' 'sickchill/oldbeard/providers/'
Length of output: 47963
sickchill/oldbeard/providers/tokyotoshokan.py (1)
52-52: LGTM! Properly internationalized debug message.
The debug message has been correctly wrapped with the _() function for internationalization, maintaining consistent logging practices.
sickchill/oldbeard/providers/newpct.py (5)
45-45: LGTM: Properly internationalized debug message
The debug message has been correctly wrapped with the _() function for internationalization, maintaining the original meaning and context.
66-66: LGTM: Properly internationalized debug message with clear context
The debug message has been correctly wrapped with the _() function, maintaining its contextual placement after the torrent rows validation.
139-139: LGTM: Properly internationalized success message
The success message has been correctly wrapped with the _() function for internationalization.
142-142: LGTM: Properly internationalized warning message
The warning message has been correctly wrapped with the _() function, maintaining its placement in the error handling path.
146-146: LGTM: Properly internationalized final failure message
The final failure message has been correctly wrapped with the _() function for internationalization.
sickchill/oldbeard/providers/torrentproject.py (2)
47-47: LGTM! Warning message properly internationalized.
The warning message for invalid custom URL is now correctly wrapped with _() for internationalization support.
55-55: LGTM! Debug message properly internationalized.
The debug message for no torrents found is now correctly wrapped with _() for internationalization support.
sickchill/oldbeard/providers/bitcannon.py (3)
26-26: LGTM: Properly internationalized warning message
The warning message for invalid custom URL is correctly wrapped with the _() function for internationalization.
49-49: LGTM: Clear debug message with proper internationalization
The debug message for no data return is correctly internationalized and provides clear feedback.
97-97: LGTM: Clear warning message with proper internationalization
The warning message for invalid API key is correctly internationalized and provides appropriate feedback.
sickchill/oldbeard/providers/torrentz.py (1)
60-60: LGTM: Proper internationalization of log messages
The log messages have been correctly wrapped with _() for internationalization.
Also applies to: 64-64
sickchill/oldbeard/providers/skytorrents.py (2)
51-51: LGTM: Proper internationalization of log message
The log message has been correctly wrapped with _() for internationalization.
51-51: Verify consistent logging patterns across providers
Let's verify if similar logging patterns exist in other provider files that might need the same improvements.
Also applies to: 80-80
✅ Verification successful
Based on the results, I can now generate the final response:
Logging patterns are consistent across providers
The logging pattern in the review comment is consistent with the codebase's general pattern. All provider files consistently:
- Use the
_()function for translation of log messages - Use string formatting with named parameters for dynamic content
- Follow similar debug logging patterns for search operations
The specific line under review follows the same pattern as hundreds of similar logging statements across the provider files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for similar logging patterns in other provider files
# Search for logger calls with format() to identify potential inconsistencies
echo "Checking for logger calls with format():"
rg --type py 'logger\.(debug|info|warning|error|exception).*format\(' sickchill/oldbeard/providers/
# Search for untranslated strings in logger calls
echo -e "\nChecking for potentially untranslated logger calls:"
rg --type py 'logger\.(debug|info|warning|error|exception)\([^_]' sickchill/oldbeard/providers/
Length of output: 31501
sickchill/oldbeard/providers/thepiratebay.py (5)
102-102: LGTM!
The debug logging statement is properly internationalized.
116-116: LGTM!
The search string debug logging is properly internationalized.
123-123: LGTM!
The URL debug logging is properly internationalized.
167-167: LGTM!
The result logging with seeder/leecher information is properly internationalized.
96-96: LGTM! Verify provider auto-disable behavior
The internationalization of the warning message is correct. The auto-disable behavior is appropriate when trackers are missing.
Let's verify if this is the only place where the provider gets disabled and if there's a way to re-enable it:
✅ Verification successful
Provider auto-disable behavior is properly implemented with re-enable option
The auto-disable behavior for ThePirateBay provider when trackers are missing is well-implemented because:
- The disable is temporary and user-fixable through the UI (config/search on torrents tab)
- The warning message clearly instructs users how to fix the issue and re-enable the provider
- The provider state is managed through the application's configuration system, allowing users to re-enable it through the web interface
- There's a proper provider management system that handles enable/disable states through
sickchill/views/config/providers.py - The system includes checks for enabled providers through
check_enabled_providers()which ensures at least one provider is available for searches
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other instances where this provider might be disabled
rg "self\.enabled\s*=\s*False" --type py
Length of output: 188
Script:
#!/bin/bash
# Let's check the context around these provider disabling instances
rg "self\.enabled\s*=\s*False" --type py -B 3 -A 3
# Also check how providers are re-enabled
rg "self\.enabled\s*=\s*True" --type py -B 3 -A 3
# Check if there's a UI or config handling for enabling/disabling providers
rg -i "enable.*provider" --type py -B 2 -A 2
Length of output: 10227
sickchill/oldbeard/providers/danishbits.py (1)
61-61: LGTM!
The internationalization of this log message is correctly implemented.
sickchill/oldbeard/providers/horriblesubs.py (2)
110-110: LGTM: Proper internationalization of debug message
The static debug message is correctly wrapped with the _() function for translation.
123-123: LGTM: Proper internationalization of debug message
The static debug message is correctly wrapped with the _() function for translation.
sickchill/oldbeard/providers/magnetdl.py (1)
60-60: LGTM!
The debug message is clear, concise, and properly internationalized.
sickchill/oldbeard/providers/btn.py (4)
41-41: LGTM: Warning message properly internationalized
The warning message for invalid API key is now correctly wrapped with the translation function.
68-68: LGTM: Debug message properly internationalized
The debug message for no data returned is now correctly wrapped with the translation function.
113-115: LGTM: API error messages properly internationalized
The warning messages for API key rejection and call limit exceeded are now correctly wrapped with the translation function.
122-122: LGTM: Timeout warning message properly internationalized
The warning message for timeout errors is now correctly wrapped with the translation function.
sickchill/oldbeard/providers/elitetorrent.py (2)
49-49: LGTM! Debug message properly internationalized.
The debug message for skipping non-Spanish shows has been correctly wrapped with _() for internationalization while maintaining appropriate logging level.
70-70: LGTM! Debug message properly internationalized.
The debug message for no torrents found has been correctly wrapped with _() for internationalization while maintaining appropriate logging level.
sickchill/oldbeard/providers/limetorrents.py (2)
45-45: LGTM: Properly internationalized log message
The log message has been correctly wrapped with the _() function for internationalization.
55-55: LGTM: Properly internationalized log message
The log message has been correctly wrapped with the _() function for internationalization.
sickchill/oldbeard/providers/kat.py (3)
75-80: LGTM! Properly internationalized error messages
The log messages have been correctly wrapped with the _() function for internationalization while maintaining clear and informative error handling.
100-100: LGTM! Properly internationalized debug message
The debug message has been correctly wrapped with the _() function for internationalization.
152-152: LGTM! Properly internationalized info message
The log message has been correctly wrapped with the _() function for internationalization.
sickchill/oldbeard/providers/ilcorsaronero.py (1)
207-207: LGTM! Error logging is properly internationalized
The error messages are clear and correctly wrapped with the _() function for internationalization.
Also applies to: 214-214
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (7)
package.json (1)
100-102: LGTM! Consider documenting the rationale for disabled rules.The disabled rules are reasonable choices:
- Disabling
unicorn/prefer-global-thisprevents overly strict enforcement ofglobalThis- Disabling
unicorn/expiring-todo-commentsis sensible as not all TODOs require expiration datesThese changes align well with the PR's goal of addressing warnings.
Consider adding comments in the config to document why these rules were disabled, making it easier for future maintainers to understand the decisions:
"rules": { "unicorn/filename-case": "off", "unicorn/prefer-node-append": "off", + // Allow using 'this' at the top level instead of forcing globalThis "unicorn/prefer-global-this": "off", + // Not all TODOs need expiration dates "unicorn/expiring-todo-comments": "off" },sickchill/gui/slick/js/core.js (3)
Line range hint
818-824: Fix the invalid querySelectorAll selectorThe current selector syntax is incorrect and will fail to select any elements. When using
querySelectorAll(), you need to use proper CSS selector syntax.Apply this fix to properly select elements by name attribute:
-const radios = document.querySelectorAll('nmjv2_dbloc'); +const radios = document.querySelectorAll('[name="nmjv2_dbloc"]');
Line range hint
818-824: Consider simplifying the radio button selection logicThe current implementation can be made more concise and efficient while maintaining the same functionality.
Here's a suggested improvement:
-const radios = document.querySelectorAll('nmjv2_dbloc'); -for (const element of radios) { - if (element.checked) { - nmjv2.dbloc = element.value; - break; - } -} +const selectedRadio = document.querySelector('[name="nmjv2_dbloc"]:checked'); +nmjv2.dbloc = selectedRadio ? selectedRadio.value : '';This refactor:
- Uses
querySelectorwith the:checkedpseudo-class to directly find the selected radio- Simplifies the value assignment with a ternary operator
- Handles the case where no radio is selected
Line range hint
818-824: Add error handling for radio button selectionThe current code lacks error handling for cases where the radio buttons might not exist or be properly configured.
Consider adding error handling:
-const radios = document.querySelectorAll('nmjv2_dbloc'); -for (const element of radios) { - if (element.checked) { - nmjv2.dbloc = element.value; - break; - } -} +try { + const selectedRadio = document.querySelector('[name="nmjv2_dbloc"]:checked'); + nmjv2.dbloc = selectedRadio ? selectedRadio.value : ''; +} catch (error) { + console.error('Failed to get NMJv2 database location:', error); + nmjv2.dbloc = ''; // Provide a safe default +}sickchill/oldbeard/providers/kat.py (3)
75-80: Improve string formatting consistencyWhile the internationalization is correct, the string formatting could be more consistent with modern practices.
Apply this diff:
- logger.info(_("{url} did not return any data, it may be disabled. Trying to get a new domain".format(url=self.url))) + logger.info(_("{url} did not return any data, it may be disabled. Trying to get a new domain").format(url=self.url))
152-152: Improve string formatting consistencyWhile the internationalization is correct, the string formatting could be more consistent with modern practices.
Apply this diff:
- logger.info(_("Setting mirror to use to {url}".format(url=self.url))) + logger.info(_("Setting mirror to use to {url}").format(url=self.url))
Line range hint
141-157: Consider enhancing mirror validation and URL handlingThe mirror selection process could benefit from additional security measures:
- Validate mirror URLs against a whitelist of known domains
- Implement HTTPS verification
- Add rate limiting for mirror requests
Would you like assistance in implementing these security enhancements?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
package.json(1 hunks)sickchill/gui/slick/js/core.js(1 hunks)sickchill/oldbeard/providers/eztv.py(2 hunks)sickchill/oldbeard/providers/hd4free.py(4 hunks)sickchill/oldbeard/providers/hdtorrents_it.py(6 hunks)sickchill/oldbeard/providers/ilcorsaronero.py(6 hunks)sickchill/oldbeard/providers/kat.py(4 hunks)sickchill/oldbeard/providers/thepiratebay.py(4 hunks)sickchill/oldbeard/providers/xthor.py(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- sickchill/oldbeard/providers/eztv.py
- sickchill/oldbeard/providers/hdtorrents_it.py
- sickchill/oldbeard/providers/xthor.py
- sickchill/oldbeard/providers/thepiratebay.py
- sickchill/oldbeard/providers/hd4free.py
- sickchill/oldbeard/providers/ilcorsaronero.py
🧰 Additional context used
🪛 Ruff (0.8.2)
sickchill/oldbeard/providers/kat.py
87-87: Local variable labels is assigned to but never used
Remove assignment to unused variable labels
(F841)
120-120: Undefined name size
(F821)
🔇 Additional comments (3)
sickchill/oldbeard/providers/kat.py (3)
87-88: Remove unused variable and fix string formatting
The log message is properly internationalized, but there are two issues to address:
- The
labelsvariable is assigned but never used - The string formatting could be improved using the
.format()method consistently
🧰 Tools
🪛 Ruff (0.8.2)
87-87: Local variable labels is assigned to but never used
Remove assignment to unused variable labels
(F841)
100-100: LGTM!
The debug message is properly internationalized and clearly describes the issue.
118-122: Fix undefined variable and improve string formatting
While the internationalization is correct, there's a critical issue:
- The
sizevariable is used in the item dictionary but is never defined - String concatenation should be replaced with
.format()
🧰 Tools
🪛 Ruff (0.8.2)
120-120: Undefined name size
(F821)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
sickchill/oldbeard/providers/kat.py (2)
75-80: Improve string formatting consistencyWhile the internationalization is correct, the string formatting could be more consistent using named parameters for better maintainability.
- logger.info(_("{url} did not return any data, it may be disabled. Trying to get a new domain".format(url=self.url))) + logger.info(_("%(url)s did not return any data, it may be disabled. Trying to get a new domain") % {"url": self.url})
Non-internationalized log messages found across provider files
Several provider files contain non-internationalized log messages that should be wrapped with the
_()function for consistency:
torrent_paradise.py: Direct f-string usage in logger callslogger.info(f"Number of torrents found on {self.name} = {len(data)}") logger.debug(f"Found result: {title} with {seeders} seeders and {leechers} leechers with a file size {size}")
thepiratebay.py: Direct string usage in logger callslogger.debug(f"Unable to process torrent on {self.name}: {error}")
scenetime.py: Non-internationalized status messageslogger.info(status)
torrentday.py: Direct status logginglogger.info(status)And many other provider files showing similar patterns
The codebase shows inconsistent internationalization of log messages, with many instances using direct strings or f-strings without the
_()wrapper. This should be standardized across all provider files for proper internationalization support.🔗 Analysis chain
Line range hint
75-152: Verify consistent internationalization across providersLet's verify that similar log messages in other provider files are also properly internationalized.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for non-internationalized log messages in provider files rg -g 'sickchill/oldbeard/providers/*.py' 'logger\.(debug|info|warning|error)\(' -A 1 | \ grep -v '_(' | \ grep -v '^--$' | \ grep -v '^[0-9]*-logger'Length of output: 39893
🧰 Tools
🪛 Ruff (0.8.2)
87-87: Local variable
labelsis assigned to but never usedRemove assignment to unused variable
labels(F841)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
sickchill/oldbeard/providers/kat.py(4 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
sickchill/oldbeard/providers/kat.py
87-87: Local variable labels is assigned to but never used
Remove assignment to unused variable labels
(F841)
120-120: Undefined name size
(F821)
🔇 Additional comments (4)
sickchill/oldbeard/providers/kat.py (4)
87-88: Remove unused variable and fix string formatting
The labels variable is assigned but never used, and the string formatting could be improved.
🧰 Tools
🪛 Ruff (0.8.2)
87-87: Local variable labels is assigned to but never used
Remove assignment to unused variable labels
(F841)
100-100: LGTM!
The debug logging is properly internationalized and provides clear information about why the torrent is being discarded.
118-122: Fix undefined variable and improve string formatting
The size variable is used in the item dictionary but is never defined, which could cause runtime errors.
🧰 Tools
🪛 Ruff (0.8.2)
120-120: Undefined name size
(F821)
152-152: LGTM!
The logging is properly internationalized and uses named parameters in the format string.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
sickchill/oldbeard/providers/kat.py (1)
Line range hint
75-152: Consider standardizing error handling patternsWhile the internationalization improvements are good, consider standardizing the error handling patterns across the provider. Some suggestions:
- Use consistent log levels (info/debug/warning) for similar types of errors
- Consider creating helper methods for common error scenarios
- Add debug logging for all major state transitions
This would make the code more maintainable and easier to troubleshoot.
🧰 Tools
🪛 Ruff (0.8.2)
87-87: Local variable
labelsis assigned to but never usedRemove assignment to unused variable
labels(F841)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (43)
sickchill/oldbeard/clients/rtorrent.py(2 hunks)sickchill/oldbeard/providers/alpharatio.py(4 hunks)sickchill/oldbeard/providers/archetorrent.py(3 hunks)sickchill/oldbeard/providers/bitcannon.py(4 hunks)sickchill/oldbeard/providers/btn.py(4 hunks)sickchill/oldbeard/providers/danishbits.py(2 hunks)sickchill/oldbeard/providers/demonoid.py(4 hunks)sickchill/oldbeard/providers/elitetorrent.py(3 hunks)sickchill/oldbeard/providers/eztv.py(2 hunks)sickchill/oldbeard/providers/filelist.py(3 hunks)sickchill/oldbeard/providers/gimmepeers.py(6 hunks)sickchill/oldbeard/providers/hd4free.py(4 hunks)sickchill/oldbeard/providers/hdspace.py(5 hunks)sickchill/oldbeard/providers/hdtorrents.py(6 hunks)sickchill/oldbeard/providers/hdtorrents_it.py(6 hunks)sickchill/oldbeard/providers/horriblesubs.py(3 hunks)sickchill/oldbeard/providers/hounddawgs.py(5 hunks)sickchill/oldbeard/providers/ilcorsaronero.py(6 hunks)sickchill/oldbeard/providers/immortalseed.py(5 hunks)sickchill/oldbeard/providers/iptorrents.py(3 hunks)sickchill/oldbeard/providers/kat.py(4 hunks)sickchill/oldbeard/providers/limetorrents.py(2 hunks)sickchill/oldbeard/providers/magnetdl.py(3 hunks)sickchill/oldbeard/providers/morethantv.py(4 hunks)sickchill/oldbeard/providers/ncore.py(4 hunks)sickchill/oldbeard/providers/nebulance.py(3 hunks)sickchill/oldbeard/providers/newznab.py(4 hunks)sickchill/oldbeard/providers/pretome.py(4 hunks)sickchill/oldbeard/providers/rarbg.py(4 hunks)sickchill/oldbeard/providers/rsstorrent.py(1 hunks)sickchill/oldbeard/providers/scc.py(3 hunks)sickchill/oldbeard/providers/scenetime.py(3 hunks)sickchill/oldbeard/providers/skytorrents.py(2 hunks)sickchill/oldbeard/providers/speedcd.py(3 hunks)sickchill/oldbeard/providers/thepiratebay.py(4 hunks)sickchill/oldbeard/providers/tntvillage.py(8 hunks)sickchill/oldbeard/providers/tokyotoshokan.py(2 hunks)sickchill/oldbeard/providers/torrentbytes.py(4 hunks)sickchill/oldbeard/providers/torrentleech.py(4 hunks)sickchill/oldbeard/providers/torrentproject.py(3 hunks)sickchill/oldbeard/providers/torrentz.py(2 hunks)sickchill/oldbeard/providers/xthor.py(3 hunks)sickchill/oldbeard/providers/yggtorrent.py(6 hunks)
✅ Files skipped from review due to trivial changes (1)
- sickchill/oldbeard/clients/rtorrent.py
🚧 Files skipped from review as they are similar to previous changes (41)
- sickchill/oldbeard/providers/rsstorrent.py
- sickchill/oldbeard/providers/skytorrents.py
- sickchill/oldbeard/providers/torrentleech.py
- sickchill/oldbeard/providers/yggtorrent.py
- sickchill/oldbeard/providers/eztv.py
- sickchill/oldbeard/providers/torrentz.py
- sickchill/oldbeard/providers/filelist.py
- sickchill/oldbeard/providers/torrentproject.py
- sickchill/oldbeard/providers/iptorrents.py
- sickchill/oldbeard/providers/hdtorrents_it.py
- sickchill/oldbeard/providers/thepiratebay.py
- sickchill/oldbeard/providers/ilcorsaronero.py
- sickchill/oldbeard/providers/pretome.py
- sickchill/oldbeard/providers/rarbg.py
- sickchill/oldbeard/providers/hd4free.py
- sickchill/oldbeard/providers/hdtorrents.py
- sickchill/oldbeard/providers/hdspace.py
- sickchill/oldbeard/providers/bitcannon.py
- sickchill/oldbeard/providers/demonoid.py
- sickchill/oldbeard/providers/tokyotoshokan.py
- sickchill/oldbeard/providers/magnetdl.py
- sickchill/oldbeard/providers/torrentbytes.py
- sickchill/oldbeard/providers/immortalseed.py
- sickchill/oldbeard/providers/horriblesubs.py
- sickchill/oldbeard/providers/newznab.py
- sickchill/oldbeard/providers/scenetime.py
- sickchill/oldbeard/providers/morethantv.py
- sickchill/oldbeard/providers/tntvillage.py
- sickchill/oldbeard/providers/elitetorrent.py
- sickchill/oldbeard/providers/speedcd.py
- sickchill/oldbeard/providers/scc.py
- sickchill/oldbeard/providers/xthor.py
- sickchill/oldbeard/providers/danishbits.py
- sickchill/oldbeard/providers/archetorrent.py
- sickchill/oldbeard/providers/alpharatio.py
- sickchill/oldbeard/providers/hounddawgs.py
- sickchill/oldbeard/providers/ncore.py
- sickchill/oldbeard/providers/nebulance.py
- sickchill/oldbeard/providers/btn.py
- sickchill/oldbeard/providers/gimmepeers.py
- sickchill/oldbeard/providers/limetorrents.py
🧰 Additional context used
🪛 Ruff (0.8.2)
sickchill/oldbeard/providers/kat.py
87-87: Local variable labels is assigned to but never used
Remove assignment to unused variable labels
(F841)
120-120: Undefined name size
(F821)
🔇 Additional comments (5)
sickchill/oldbeard/providers/kat.py (5)
75-80: LGTM: Error handling and logging improvements
The error handling for failed data retrieval is comprehensive and the log messages are properly internationalized using the _() function with correct format string usage.
87-88: Remove unused variable and improve string formatting
The log message needs improvement and there's an unused variable.
Apply this diff:
- labels = [cell.get_text() for cell in html.find(class_="firstr")("th")]
- logger.info(_("Found {} results").format(len(html("tr", **self.rows_selector))))
+ logger.info(_("Found {count} results").format(count=len(html("tr", **self.rows_selector))))🧰 Tools
🪛 Ruff (0.8.2)
87-87: Local variable labels is assigned to but never used
Remove assignment to unused variable labels
(F841)
100-100: LGTM: Clear debug logging
The debug message is clear and properly internationalized.
152-152: LGTM: Clear mirror URL logging
The log message for setting the mirror URL is clear and properly internationalized.
118-122:
Fix undefined variable and improve string formatting
There are issues with undefined variables and string formatting consistency.
Apply this diff:
- logger.debug(_("Found result {title} but that doesn't seem like a verified result so I'm ignoring it").format(title=title))
- item = {"title": title, "link": download_url, "size": size, "seeders": seeders, "leechers": leechers, "hash": torrent_hash}
+ # Get size from the result row
+ size = convert_size(result.find_all("td")[1].get_text(strip=True)) or -1
+ item = {"title": title, "link": download_url, "size": size, "seeders": seeders, "leechers": leechers, "hash": torrent_hash}
- logger.debug(_("Found result: {0} with {1} seeders and {2} leechers").format(title, seeders, leechers))
+ logger.debug(_("Found result: {title} with {seeders} seeders and {leechers} leechers").format(
+ title=title, seeders=seeders, leechers=leechers
+ ))🧰 Tools
🪛 Ruff (0.8.2)
120-120: Undefined name size
(F821)
Fixes 22-warning-log-no-provider
Summary by CodeRabbit
New Features
Bug Fixes
searchmethod for several providers, ensuring better tracking of search processes and connection issues.Documentation