-
-
Notifications
You must be signed in to change notification settings - Fork 601
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
root dirs, trakt, addshow, bdecode #8718
Conversation
WalkthroughThese changes enhance the codebase by simplifying method signatures, internalizing arguments, and improving efficiency and readability when interacting with external services and configurations. The focus is on streamlining show management, adding and fetching trending shows, and initializing server settings. Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this 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
Files selected for processing (3)
- sickchill/oldbeard/traktChecker.py (2 hunks)
- sickchill/views/manage/add_shows.py (5 hunks)
- sickchill/views/server_settings.py (1 hunks)
Files skipped from review due to trivial changes (1)
- sickchill/views/server_settings.py
Additional comments: 5
sickchill/views/manage/add_shows.py (4)
- 43-47: The method
searchIndexersForShowName
has been modified to handlesearch_term
internally by retrieving it from the body of the request. This change simplifies the method's interface by removing the need for external argument passing. However, ensure that all calls to this method have been updated accordingly to reflect this change.Verification successful
The verification process confirms that the method
searchIndexersForShowName
inadd_shows.py
is not called with any arguments throughout the codebase. This aligns with the changes made to the method to internally retrieve thesearch_term
from the request body, indicating that there is no need for concern regarding the update of calls to this method.* 114-114: The `massAddTable` method no longer accepts `rootDir` as an argument and instead retrieves it from the query parameters. This change aligns with the objective of simplifying method interfaces. Similar to the previous comment, verify that all invocations of `massAddTable` have been updated to not pass `rootDir` as an argument.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for occurrences of `searchIndexersForShowName` to ensure no arguments are passed. rg "searchIndexersForShowName\(" --type pyLength of output: 121
Verification successful
Given the search results, which only show the definition of
massAddTable
without any direct invocations passing arguments, it appears that the method's interface change has been correctly handled across the codebase. This suggests that all necessary updates to method invocations have been made in alignment with the objective of simplifying method interfaces. However, it's important to note that this verification covers direct invocations and might not capture dynamic or indirect method calls, which are common in web applications. Based on the provided evidence, the review comment's concern seems to have been addressed.* 242-242: The `trendingShows` method has been updated to no longer accept `traktList` as an argument, instead retrieving it from the query parameters. This change is consistent with the PR's objective to streamline parameter handling. Ensure that the frontend or any API calls to this method have been updated to reflect this change.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for occurrences of `massAddTable` to ensure no arguments are passed. rg "massAddTable\(" --type pyLength of output: 95
Verification successful
The verification process supports the review comment's claim that the
trendingShows
method has been updated to no longer accepttraktList
as an argument, instead retrieving it from the query parameters. The search did not find any calls to this method passing arguments, which suggests that the frontend or any API calls to this method have been updated accordingly. However, it's important to note that this verification is based on static analysis and may not capture dynamic or indirect method calls.* 269-269: The `getTrendingShows` method has been updated similarly to `trendingShows`, with `traktList` now being retrieved from the query parameters instead of being passed as an argument. This modification enhances consistency in how parameters are handled across methods. As with previous changes, verify that all calls to `getTrendingShows` have been updated accordingly.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for occurrences of `trendingShows` to ensure no arguments are passed. rg "trendingShows\(" --type pyLength of output: 97
Script:
#!/bin/bash # Search for calls to `trendingShows` method to ensure no arguments are being passed. rg "trendingShows\(" --type pyLength of output: 97
Verification successful
Based on the script's output, it appears that the
getTrendingShows
method is defined without accepting any arguments, and there are no calls to this method with arguments within the codebase. This aligns with the modifications mentioned in the review comment, ensuring consistency in how parameters are handled across methods. Therefore, it can be concluded that the changes togetTrendingShows
have been correctly implemented.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for occurrences of `getTrendingShows` to ensure no arguments are passed. rg "getTrendingShows\(" --type pyLength of output: 103
sickchill/oldbeard/traktChecker.py (1)
- 24-24: The removal of the
force
parameter from therun
method simplifies the method signature. However, ensure that all calls to this method throughout the codebase have been updated to reflect this change. If theforce
parameter was previously used to control the execution flow withinrun
, consider verifying that its removal does not affect the intended functionality.
There was a problem hiding this 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
Files selected for processing (2)
- sickchill/oldbeard/traktChecker.py (1 hunks)
- sickchill/views/manage/add_shows.py (6 hunks)
Files skipped from review as they are similar to previous changes (2)
- sickchill/oldbeard/traktChecker.py
- sickchill/views/manage/add_shows.py
There was a problem hiding this 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
Files selected for processing (1)
- sickchill/oldbeard/clients/generic.py (1 hunks)
Additional comments: 1
sickchill/oldbeard/clients/generic.py (1)
- 164-164: The change from using
b"info"
to"info"
for accessing theinfo
field in thetorrent_bdecode
dictionary is correct if the dictionary keys are indeed strings rather than bytes. This adjustment is crucial for ensuring compatibility with the expected data types in Python 3, where strings and bytes are distinct types. However, it's important to verify that all instances wheretorrent_bdecode
is used or populated have been updated to maintain consistency with this change. If there are other places in the codebase wheretorrent_bdecode
is manipulated or accessed, those should also use string keys to avoid key errors.Verification successful
The verification process confirms that the change from using
b"info"
to"info"
for accessing theinfo
field in thetorrent_bdecode
dictionary is consistent across the codebase, as there are no other occurrences oftorrent_bdecode
being accessed or modified. This ensures compatibility with the expected data types in Python 3 and maintains consistency within the codebase.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other occurrences of torrent_bdecode being accessed or modified to ensure consistency. rg "torrent_bdecode\["Length of output: 109
@@ -70,7 +70,7 @@ def __init__(self, options=None): | |||
# video root | |||
if settings.ROOT_DIRS: | |||
root_dirs = settings.ROOT_DIRS.split("|") | |||
self.video_root = root_dirs[int(root_dirs[0]) + 1] | |||
self.video_root = root_dirs[int(root_dirs[0])] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why change this one? we dont use video_root
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first directory location no longer stored as 0 but 1 so adding 1 to 1 when only 1 entry errors at startup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yeah I was seeing the diff in reverse. That little section needs removed, video_root was a strange thing echelon added at some point where your videos could be watched remotely without a password even, it didn't require authentication. Idk if he was scraping other people's videos or what.
Fixes
Summary by CodeRabbit
traktChecker
class to enhance the show tracking process without requiring aforce
update.