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

Generic Provide + Notifications #8691

Merged
merged 16 commits into from
Feb 1, 2024
Merged

Generic Provide + Notifications #8691

merged 16 commits into from
Feb 1, 2024

Conversation

BKSteve
Copy link
Collaborator

@BKSteve BKSteve commented Jan 16, 2024

Fix Generic Provider episode_object error
#8688

Summary by CodeRabbit

  • New Features
    • Added DISCORD_WEBHOOK variable to settings for enhanced Discord integration.
    • Updated URL for "opensubtitles" source to use HTTPS for improved security.
  • Bug Fixes
    • Corrected the logic in the get_coming_episodes function to ensure it always returns results.
  • Refactor
    • Simplified Discord notification logic by removing the tts parameter and related settings.
    • Improved error logging in Discord notification sending.
  • Chores
    • Removed the "Discord TTS" checkbox from the GUI to streamline notification settings.
  • Style
    • Adjusted search results retrieval logic in GenericProvider.py for clarity and efficiency.

Copy link
Contributor

coderabbitai bot commented Jan 16, 2024

Warning

Rate Limit Exceeded

@BKSteve has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 14 minutes and 40 seconds before requesting another review.

How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.
Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.
Please see our FAQ for further information.

Commits Files that changed from the base of the PR and between eef3235 and 2e83e57.

Walkthrough

The recent updates to SickChill involve refining the Discord notification system by removing the text-to-speech (TTS) option and enhancing error logging. Additionally, there's an adjustment in torrent argument formatting, a shift in handling search results for media, an update to the OpenSubtitles URL, and a modification in the logic for upcoming episodes. These changes streamline user settings, improve consistency, and update external links, reflecting a focus on user experience and application reliability.

Changes

File Path Change Summary
sickchill/providers/GenericProvider.py Removed episode_object parameter from self.search in find_search_results.
sickchill/oldbeard/clients/qbittorrent.py Fixed spacing in ratio_limit assignment in __torrent_args.
sickchill/gui/slick/js/core.js, .../views/config_notifications.mako Removed discord.tts parameter from a $.get request and associated UI checkbox.
sickchill/oldbeard/notifications_queue.py, .../notifiers/discord.py Simplified Discord notification logic by removing tts. Improved error logging.
sickchill/settings.py, start.py, .../config/notifications.py, .../views/home.py Updated Discord settings handling, including removal of DISCORD_TTS and addition of webhook settings.
sickchill/oldbeard/subtitles.py Updated OpenSubtitles URL to use HTTPS.
sickchill/show/ComingEpisodes.py, .../views/index.py Modified logic for episode retrieval and added error handling in await self.finish(results).

"In the land of code and wire,
Changes swift as rabbits desire.
🐇 With each tweak and turn, we aspire,
To make SickChill soar ever higher!"

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: 1

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between baf3c28 and 981b270.
Files selected for processing (1)
  • sickchill/providers/GenericProvider.py (1 hunks)

sickchill/providers/GenericProvider.py Show resolved Hide resolved
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 981b270 and f7891aa.
Files selected for processing (1)
  • sickchill/oldbeard/clients/qbittorrent.py (1 hunks)
Files skipped from review due to trivial changes (1)
  • sickchill/oldbeard/clients/qbittorrent.py

@BKSteve BKSteve changed the title Generic p Generic Provide + Notifications Jan 21, 2024
@BKSteve
Copy link
Collaborator Author

BKSteve commented Jan 21, 2024

Remove Discord TTS setting and code as it doesn't TTS and message rather than speech is sufficient notification.
Otherwise requires much bigger rewrite.

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 f7891aa and 330bb97.
Files selected for processing (8)
  • sickchill/gui/slick/js/core.js (1 hunks)
  • sickchill/gui/slick/views/config_notifications.mako (1 hunks)
  • sickchill/oldbeard/notifications_queue.py (3 hunks)
  • sickchill/oldbeard/notifiers/discord.py (1 hunks)
  • sickchill/settings.py (1 hunks)
  • sickchill/start.py (2 hunks)
  • sickchill/views/config/notifications.py (2 hunks)
  • sickchill/views/home.py (1 hunks)
Additional comments: 10
sickchill/oldbeard/notifiers/discord.py (1)
  • 38-42: The test_notify function signature has been updated to remove the tts parameter, which is consistent with the PR objectives and AI-generated summary. The function body has been updated accordingly, and no references to the removed parameter remain.
sickchill/oldbeard/notifications_queue.py (1)
  • 147-158: > 📝 NOTE

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

The _send_discord method has been refactored to remove the tts parameter and the discord_tts variable. The JSON payload for the POST request has been simplified by using a message_data variable. Error handling has been improved to provide more detailed information, which is a good practice for debugging and logging purposes.

sickchill/views/home.py (1)
  • 421-421: The testDiscord function has been modified to remove the tts parameter when calling notifiers.discord_notifier.test_notify. Ensure that the notifiers.discord_notifier.test_notify method has been updated accordingly to handle the absence of the tts parameter.
sickchill/start.py (1)
  • 607-612: > 📝 NOTE

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

The removal of settings.DISCORD_TTS from the path_leaf and save_config functions is consistent with the PR's objective to update Discord settings handling. Ensure that no other parts of the application rely on settings.DISCORD_TTS, as this could lead to runtime errors if it's expected elsewhere.

sickchill/gui/slick/js/core.js (5)
  • 716-728: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]

The file core.js contains JavaScript code for the SickChill GUI, including functions and event handlers.

  • 716-728: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]

The utility functions such as getMeta, metaToBool, and isMeta are used throughout the file to retrieve and interpret meta tag values.

  • 716-728: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]

Event handlers are attached to various DOM elements to handle user interactions such as clicks, changes, and form submissions.

  • 716-728: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]

DOM manipulation is performed to update the UI dynamically based on user actions or other events, such as showing or hiding elements and updating text content.

  • 716-728: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]

Initialization code is present to set up the page when it loads, including setting up table sorters and loading content dynamically.

sickchill/gui/slick/views/config_notifications.mako (1)
  • 3764-3769: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]

The file is a Mako template for configuring notification settings in the SickChill application.

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 330bb97 and d9c7dbf.
Files selected for processing (1)
  • sickchill/oldbeard/subtitles.py (1 hunks)
Files skipped from review due to trivial changes (1)
  • sickchill/oldbeard/subtitles.py

@BKSteve
Copy link
Collaborator Author

BKSteve commented Jan 21, 2024

Add in change of http://www.opensubtitles.org to https://www.opensubtitles.com

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 d9c7dbf and f57c7bb.
Files selected for processing (1)
  • sickchill/show/ComingEpisodes.py (1 hunks)
Additional comments: 1
sickchill/show/ComingEpisodes.py (1)
  • 142-147: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-112]

The removal of the else block that returned an empty list from the get_coming_episodes function changes the control flow. Ensure that all calling functions are capable of handling the grouped_results even when the result set is empty, as this could potentially introduce bugs if not handled correctly.

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: 2

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between f57c7bb and cdff515.
Files selected for processing (8)
  • sickchill/gui/slick/js/core.js (1 hunks)
  • sickchill/gui/slick/views/config_notifications.mako (2 hunks)
  • sickchill/oldbeard/notifications_queue.py (2 hunks)
  • sickchill/settings.py (1 hunks)
  • sickchill/start.py (2 hunks)
  • sickchill/views/config/notifications.py (2 hunks)
  • sickchill/views/home.py (1 hunks)
  • sickchill/views/index.py (1 hunks)
Files skipped from review as they are similar to previous changes (7)
  • sickchill/gui/slick/js/core.js
  • sickchill/gui/slick/views/config_notifications.mako
  • sickchill/oldbeard/notifications_queue.py
  • sickchill/settings.py
  • sickchill/start.py
  • sickchill/views/config/notifications.py
  • sickchill/views/home.py
Additional comments: 2
sickchill/views/index.py (2)
  • 175-184: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]

The imports in this file are well-organized and follow Python's best practices. However, ensure that all imported modules are used within the file to avoid unnecessary imports which can lead to confusion and slightly increased memory usage.

  • 175-184: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [7-7]

The use of ThreadPoolExecutor within WebHandler is a good approach for handling asynchronous tasks. Ensure that the executor is properly managed and shut down when not in use to prevent resource leaks.

sickchill/views/index.py Show resolved Hide resolved
@BKSteve
Copy link
Collaborator Author

BKSteve commented Jan 26, 2024

Discord TTS big change fixes the TTS setting and realigns it and improves selection to enable the whole message or just the bot name to be sent.
The Bot name can be anything you want as actual bot is based on web hook. It just can't be blank so it gets forced to SickChill if blank.

With this testing have added in index.py try around await self.finish(results) as was throwing an exception with all shows regarding prowl_notify_list which core.js says is an Unresolved variable on 1228.
Need some assistance with this js please.

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: 1

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between cdff515 and 19ac68e.
Files selected for processing (10)
  • SickChill.py (1 hunks)
  • sickchill/providers/subtitle/bsplayer.py (1 hunks)
  • sickchill/show/recommendations/recommended.py (1 hunks)
  • sickchill/tagger/episode.py (1 hunks)
  • sickchill/views/server_settings.py (2 hunks)
  • tests/sickchill_tests/show/test_coming_episodes.py (1 hunks)
  • tests/sickchill_tests/show/test_show.py (1 hunks)
  • tests/test_db.py (1 hunks)
  • tests/test_notifier.py (1 hunks)
  • tests/test_post_processor_queue.py (1 hunks)
Files skipped from review due to trivial changes (5)
  • SickChill.py
  • tests/sickchill_tests/show/test_coming_episodes.py
  • tests/sickchill_tests/show/test_show.py
  • tests/test_db.py
  • tests/test_notifier.py
Additional comments: 8
sickchill/show/recommendations/recommended.py (2)
  • 4-4: The addition of the posixpath module suggests changes in path handling, likely for cross-platform compatibility or URL path manipulation. Ensure that its usage aligns with the intended file system or URL path operations across different operating systems.
  • 1-7: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [52-52]

The use of posixpath.join for constructing URL-like paths in cache_image is appropriate, ensuring compatibility and standardization of path formats in web contexts.

tests/test_post_processor_queue.py (2)
  • 4-4: The addition of the datetime module suggests new or expanded use of datetime functionality in the test cases. Verify that datetime operations are correctly implemented, especially in time-sensitive tests.
  • 1-7: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [26-26]

The use of datetime.timedelta in setUp for configuring run_delay and cycleTime is appropriate for testing time-dependent functionalities of the post processor queue.

sickchill/tagger/episode.py (2)
  • 4-4: The addition of the re (regular expressions) module indicates new or expanded use of regex for parsing episode information. Ensure regex patterns are correctly designed and tested for the intended parsing tasks.
  • 1-7: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [17-17]

The implementation of regex for extracting media-related tags in EpisodeTags is correctly handled, demonstrating appropriate usage of compiled patterns and match object checks.

sickchill/providers/subtitle/bsplayer.py (1)
  • 4-4: The summary mentions an added import statement without specifying the module. Ensure any new dependencies are appropriately integrated and used within the module's functionality.
sickchill/views/server_settings.py (1)
  • 138-138: Modification to use SickChillStaticFileHandler as the static_handler_class suggests enhancements in static file handling. Verify that this custom handler correctly implements any intended customizations, such as URL versioning or dynamic path resolution.

sickchill/views/server_settings.py Show resolved Hide resolved
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 19ac68e and 589fb32.
Files selected for processing (1)
  • sickchill/gui/slick/js/core.js (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • sickchill/gui/slick/js/core.js

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 589fb32 and eef3235.
Files selected for processing (2)
  • sickchill/gui/slick/js/core.js (1 hunks)
  • sickchill/oldbeard/notifications_queue.py (2 hunks)
Files skipped from review as they are similar to previous changes (2)
  • sickchill/gui/slick/js/core.js
  • sickchill/oldbeard/notifications_queue.py

@miigotu miigotu merged commit 42670d9 into develop Feb 1, 2024
11 checks passed
@miigotu miigotu deleted the GenericP branch February 1, 2024 03:39
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.

2 participants