Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: lots of refactoring, try to use a set for search strings to avoid duplicates, add type checking for search strings, untested #8704

Merged
merged 1 commit into from
Feb 2, 2024

Conversation

miigotu
Copy link
Contributor

@miigotu miigotu commented Feb 2, 2024

Summary by CodeRabbit

  • New Features
    • Added type hints across various provider classes for improved code clarity and maintenance.
    • Enhanced search functionality with the introduction of only_spanish_search attribute for more focused search results.
  • Enhancements
    • Updated URLs to use HTTPS for enhanced security.
    • Improved readability of logging messages by adopting f-strings across the application.
    • Streamlined authentication processes within provider classes.
  • Bug Fixes
    • Corrected grammar and clarified comments in code and documentation.
    • Fixed regex patterns and method signatures for better functionality and consistency.
  • Refactor
    • Replaced bencodepy library usage with bencode for more efficient torrent data handling.
    • Standardized method naming and variable naming conventions for greater code readability and maintainability.

… avoid duplicates, add type checking for search strings, untested

Signed-off-by: miigotu <miigotu@gmail.com>
Copy link
Contributor

coderabbitai bot commented Feb 2, 2024

Walkthrough

The recent updates to SickChill focus on improving code readability, consistency, and security. Changes include the adoption of f-strings for better formatting, renaming variables for clarity, updating method signatures with type hints, and switching from bencodepy to bencode for torrent handling. Additionally, there's a shift towards using HTTPS URLs and refining authentication methods across various providers, alongside minor adjustments in documentation and logging messages.

Changes

Files Change Summary
config_providers.mako, .../providers/..., start.py, views/config/providers.py Renamed onlyspasearch to only_spanish_search for clarity.
.../download_station.py, .../generic.py, .../providers/..., update_manager/runner.py, views/api/webapi.py, tests/... Updated to use f-strings for better readability.
.../providers/generic.py, TorrentProvider.py Replaced bencodepy with bencode for torrent data handling.
.../helpers.py Removed an empty line.
.../notifications_queue.py Adjusted documentation and logging related to post-processing and Discord.
.../providers/... Modified authentication method names and logic, updated URLs to HTTPS, and improved regex patterns.
.../providers/..., GenericProvider.py, FrenchProvider.py Added type hints and updated method signatures.

"In the land of code and night,
🐇 A rabbit worked to make things right.
With a hop and a skip, updates took flight,
Ensuring SickChill shone ever bright."
🌟🌌🔧

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?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit-tests for this file.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit tests for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository from git and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit tests.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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 as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between fc40d49 and 0b13f9a.
Files selected for processing (30)
  • sickchill/gui/slick/views/config_providers.mako (1 hunks)
  • sickchill/oldbeard/clients/download_station.py (1 hunks)
  • sickchill/oldbeard/clients/generic.py (2 hunks)
  • sickchill/oldbeard/helpers.py (1 hunks)
  • sickchill/oldbeard/notifications_queue.py (3 hunks)
  • sickchill/oldbeard/providers/bitcannon.py (2 hunks)
  • sickchill/oldbeard/providers/bjshare.py (4 hunks)
  • sickchill/oldbeard/providers/btn.py (5 hunks)
  • sickchill/oldbeard/providers/elitetorrent.py (4 hunks)
  • sickchill/oldbeard/providers/hd4free.py (1 hunks)
  • sickchill/oldbeard/providers/hdbits.py (6 hunks)
  • sickchill/oldbeard/providers/ilcorsaronero.py (2 hunks)
  • sickchill/oldbeard/providers/immortalseed.py (2 hunks)
  • sickchill/oldbeard/providers/morethantv.py (7 hunks)
  • sickchill/oldbeard/providers/newpct.py (5 hunks)
  • sickchill/oldbeard/providers/newznab.py (2 hunks)
  • sickchill/oldbeard/providers/norbits.py (2 hunks)
  • sickchill/oldbeard/providers/omgwtfnzbs.py (2 hunks)
  • sickchill/oldbeard/providers/rarbg.py (4 hunks)
  • sickchill/oldbeard/providers/rsstorrent.py (2 hunks)
  • sickchill/oldbeard/providers/shazbat.py (2 hunks)
  • sickchill/providers/GenericProvider.py (5 hunks)
  • sickchill/providers/torrent/FrenchProvider.py (4 hunks)
  • sickchill/providers/torrent/TorrentProvider.py (2 hunks)
  • sickchill/start.py (2 hunks)
  • sickchill/update_manager/runner.py (1 hunks)
  • sickchill/views/api/webapi.py (1 hunks)
  • sickchill/views/config/providers.py (1 hunks)
  • tests/test_helpers.py (1 hunks)
  • tests/test_xem.py (1 hunks)
Files not reviewed due to errors (1)
  • (no review received)
Files skipped from review due to trivial changes (7)
  • sickchill/oldbeard/clients/download_station.py
  • sickchill/oldbeard/helpers.py
  • sickchill/oldbeard/providers/bjshare.py
  • sickchill/oldbeard/providers/hd4free.py
  • sickchill/views/api/webapi.py
  • tests/test_helpers.py
  • tests/test_xem.py
Additional comments: 61
sickchill/oldbeard/providers/shazbat.py (2)
  • 33-33: Method check_auth_from_data has been renamed from _check_auth_from_data and its accessibility changed. Ensure that all references to this method within the class and externally (if any) are updated accordingly.
  • 49-49: The method call self.provider.check_auth_from_data(data) correctly reflects the updated method name. This change ensures consistency with the method's new visibility and naming convention.
sickchill/oldbeard/providers/omgwtfnzbs.py (2)
  • 28-28: Method check_auth_from_data has been correctly renamed from _check_auth_from_data and its accessibility changed. This modification aligns with the objective to streamline the authentication check process within the provider class.
  • 78-78: The method call self.check_auth_from_data(data, is_XML=False) accurately reflects the updated method name, ensuring that the authentication check process is correctly invoked with the new method signature.
sickchill/providers/torrent/FrenchProvider.py (3)
  • 2-2: The addition of type hints using typing for function parameters and return types enhances code readability and maintainability by specifying expected data types.
  • 88-90: Properties url and custom_url are defined using property decorators, which is a best practice for encapsulating attribute access and ensuring that any logic associated with getting or setting these attributes is correctly managed.
  • 85-97: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [93-105]

The get_season_search_strings method now correctly expects a TVEpisode object and returns a list of dictionaries, aligning with the objective to introduce type hints and improve the method's return type clarity.

sickchill/providers/torrent/TorrentProvider.py (2)
  • 3-3: Updating the import statements from bencodepy to bencode for handling torrent file decoding is a significant change. This update affects the usage of the bencode library in the file, which is crucial for handling torrent data correctly.
  • 108-109: The method _verify_download correctly uses bencode.bread(filename) to validate torrent files, reflecting the updated import from bencode. This change ensures that torrent files are correctly decoded and validated using the bencode library.
sickchill/oldbeard/providers/bitcannon.py (2)
  • 52-52: The method call self.check_auth_from_data(parsed_json) correctly reflects the updated method name check_auth_from_data, ensuring that the authentication check process is accurately invoked within the search method.
  • 95-95: Method check_auth_from_data has been correctly changed to a static method with some logic adjustments related to API key validation. This change is consistent with the objective to modify the method call within the search method.
sickchill/oldbeard/providers/norbits.py (2)
  • 35-35: Method check_auth_from_data has been correctly renamed from _check_auth_from_data, affecting its invocation within the class methods. This change ensures that the method is accessible in a manner consistent with its intended use case.
  • 69-69: The method call self.check_auth_from_data(parsed_json) accurately reflects the updated method name, ensuring that the authentication check process is correctly invoked within the search method.
sickchill/oldbeard/providers/hdbits.py (6)
  • 3-3: Adding type hints using typing for function parameters and return types enhances code readability and maintainability by specifying expected data types.
  • 34-34: Changing the method check_auth_from_data to a static method is a significant modification that affects its invocation within the class methods. Ensure that all references to this method are updated accordingly.
  • 42-47: The method signatures for get_season_search_strings and get_episode_search_strings have been updated to use type hints and annotations, improving the clarity and maintainability of the code.
  • 67-67: The method call self.check_auth_from_data(parsed_json) correctly reflects the updated method name and static nature, ensuring that the authentication check process is accurately invoked within the search method.
  • 85-85: Renaming make_post_data_JSON to make_post_data_json and updating method calls to use lowercase json instead of JSON aligns with Python naming conventions and improves code readability.
  • 146-146: The method call self.provider.get_url(self.provider.urls["rss"], post_data=self.provider.make_post_data_json(), returns="json") correctly reflects the updated method name make_post_data_json, ensuring that the RSS data retrieval process is accurately invoked.
sickchill/oldbeard/notifications_queue.py (4)
  • 17-17: Semantic adjustments in the documentation clarify the purpose of the NotificationsQueue class, specifically highlighting its role in handling multiple post-processing tasks.
  • 31-31: The documentation update clarifies the functionality of the is_paused property, enhancing understanding of how the post-processing queue's paused state is determined.
  • 163-163: Logging messages related to Discord interactions have been semantically adjusted to provide clearer information about the notification process, specifically regarding webhook connectivity issues.
  • 170-171: The handling of Discord rate limiting with a specific log message and retry mechanism after a calculated delay improves the robustness of Discord notifications, ensuring messages are sent even under rate limiting conditions.
sickchill/oldbeard/providers/rsstorrent.py (2)
  • 4-4: Updating the import statement from bencodepy to bencode and adjusting the corresponding function calls to use bencode instead of bencodepy is a significant change that affects the decoding of torrent files within the validateRSS method.
  • 145-146: The method validateRSS correctly uses bencode.decode(torrent_file) to validate torrent files, reflecting the updated import from bencode. This change ensures that torrent files are correctly decoded and validated using the bencode library.
sickchill/oldbeard/providers/immortalseed.py (2)
  • 49-49: Method check_auth_from_data has been correctly renamed from _check_auth_from_data, making it accessible externally. This change affects the method call within _check_auth by updating the method name accordingly.
  • 171-171: The method call self.provider.check_auth_from_data(data) accurately reflects the updated method name check_auth_from_data, ensuring that the authentication check process is correctly invoked within the _check_auth method.
sickchill/oldbeard/providers/elitetorrent.py (4)
  • 17-17: Renaming self.onlyspasearch to self.only_spanish_search improves readability and adheres to Python naming conventions for variable names, making the code more understandable.
  • 48-48: The conditional check using self.only_spanish_search correctly reflects the updated variable name, ensuring that the search functionality respects the intended language preference.
  • 99-99: Method _process_title has been correctly renamed from _processTitle, aligning with Python naming conventions for method names and improving code readability.
  • 143-143: The static method _process_title correctly processes the title to include quality, language, and provider information, enhancing the clarity and usefulness of the title for users.
sickchill/oldbeard/providers/rarbg.py (3)
  • 4-4: The addition of type hints for TYPE_CHECKING and the update of a URL to use HTTPS are good practices that enhance code readability, maintainability, and security.
  • 31-31: The update to the API URL to use HTTPS instead of HTTP is a critical security improvement, ensuring that all communications with the API are encrypted.
  • 156-162: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [159-170]

The get_season_search_strings method now correctly expects a TVEpisode object and returns a list of dictionaries, aligning with the objective to introduce type hints and improve the method's return type clarity.

sickchill/oldbeard/providers/morethantv.py (4)
  • 2-2: Added type hints for Dict, List, and TYPE_CHECKING improve code readability and maintainability by specifying expected data types.
  • 85-90: Refactoring process_column_header to use column_result instead of result clarifies the variable's purpose and improves readability.
  • 100-100: Introduction of searchedSeason variable enhances readability by clearly indicating its purpose within the search logic.
  • 198-205: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [185-203]

Updating get_season_search_strings to use a set for search strings is a performance improvement, as sets offer faster lookups compared to lists, especially beneficial in search operations.

sickchill/oldbeard/providers/btn.py (4)
  • 5-5: Added type hints for Dict, Iterable, List, TYPE_CHECKING, and Union enhance code readability and maintainability by specifying expected data types.
  • 44-44: Renaming _check_auth_from_data to check_auth_from_data and making it a static method improves clarity and indicates that the method does not depend on instance variables.
  • 186-197: Modified method signatures for get_season_search_strings and get_episode_search_strings to accept a TVEpisode object and return specific types, improving type safety and clarity.
  • 199-218: Updated references from episode_object to episode within the get_season_search_strings and get_episode_search_strings methods, enhancing consistency and readability.
sickchill/oldbeard/providers/newpct.py (6)
  • 15-15: Adding a boolean attribute only_spanish_search with a default value improves configurability, allowing for more targeted search operations based on language preference.
  • 17-17: Updating the provider URL to use HTTPS enhances security by ensuring encrypted communication with the provider.
  • 41-48: Adjusted search parameters based on the only_spanish_search attribute, improving search efficiency and relevance by filtering results based on language preference.
  • 71-78: Refactored the logic for setting search parameters and search mode conditions, enhancing code readability and maintainability.
  • 128-128: Improved the handling of downloading torrent files by parsing the show sheet to access the torrent, addressing the indirect torrent file access issue.
  • 151-156: Enhanced the title processing method _process_title by removing unnecessary literals and adjusting spacing, improving title readability and consistency.
sickchill/oldbeard/clients/generic.py (2)
  • 9-9: Switching from bencodepy to bencode for decoding and encoding torrent data could potentially improve performance and compatibility. Ensure that bencode is properly installed and tested across the application to avoid runtime errors.
  • 156-176: The handling of torrent data decoding and encoding, including error logging and hashing operations, has been updated to use the bencode library. Ensure that these changes are thoroughly tested, especially the error handling and hash generation, to maintain functionality and error resilience.
sickchill/update_manager/runner.py (1)
  • 71-71: Updating the logging statement to use an f-string for formatting the error message improves readability and consistency with modern Python practices.
sickchill/views/config/providers.py (1)
  • 245-245: Renaming the option from "onlyspasearch" to "only_spanish_search" in the saveProviders function improves readability and consistency with naming conventions.
sickchill/oldbeard/providers/ilcorsaronero.py (3)
  • 173-173: The URL in the _magnet_from_result method has been updated to use HTTPS, enhancing security.
  • 295-295: The logging format in the search method has been updated to use f-strings, improving readability and performance.
  • 295-295: Ensure the sorting logic adjustment for items by seeders in the search method aligns with the intended performance improvements and correctness.
sickchill/oldbeard/providers/newznab.py (2)
  • 197-197: The method _check_auth_from_data has been renamed to check_auth_from_data and updated to be a non-static method, potentially affecting authentication logic. Ensure all references to this method have been updated accordingly.
Verification successful

The search results indicate that there are no references to the old method name _check_auth_from_data in the codebase, suggesting that all necessary updates to the method name have been made. The new method name check_auth_from_data is used across various files in the sickchill/oldbeard/providers directory, indicating that the method renaming has been acknowledged and implemented across the relevant parts of the application. This includes direct method calls within the same file (newznab.py) and calls from other provider files, which suggests a consistent update to accommodate the method's new signature and accessibility as a non-static method.

Given the evidence from the script output, it appears that the renaming of _check_auth_from_data to check_auth_from_data and its update to a non-static method have been properly handled within the codebase, with all references to this method updated accordingly.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Search for old method name to ensure all references have been updated.
rg --type py "_check_auth_from_data"
# Search for new method name to validate its usage.
rg --type py "check_auth_from_data"

Length of output: 1636

* 325-325: The updated method `check_auth_from_data` is now being correctly called within the `search` method. This change should be verified to ensure it doesn't impact the authentication process negatively.
sickchill/providers/GenericProvider.py (3)
  • 8-8: Type annotations Dict, Iterable, List, and Union have been added to the imports, aligning with the Python typing standards for better code readability and maintainability.
  • 416-426: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [419-451]

The get_episode_search_strings method now returns a Union[List[Dict], Iterable] and uses a set for the Episode search strings to eliminate duplicates, enhancing efficiency. Ensure that the consuming functions are compatible with these changes.

  • 446-459: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [455-470]

The get_season_search_strings method has been updated to use a set for the Season search strings, which is a good practice to avoid duplicate search strings and potentially improve search efficiency.

sickchill/gui/slick/views/config_providers.mako (1)
  • 532-532: The variable name change from onlyspasearch to only_spanish_search improves readability and follows Python's naming conventions for variables. This change aligns with the PR's objective to enhance code clarity. Ensure that all references to this variable in the backend logic are also updated to reflect the new name.

@miigotu miigotu merged commit 7ae5a29 into develop Feb 2, 2024
12 checks passed
@miigotu miigotu deleted the spazout branch February 2, 2024 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant