-
-
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
QBT remove false link to torrent folder. #8628
Conversation
Note Reviews PausedUse the following commands to manage reviews:
WalkthroughThe recent updates to the SICKCHILL codebase focus on enhancing the integration with qBittorrent and refining the logging system. Changes include adjustments to torrent path settings, improved visibility of file browser options, and a shift from info to debug logging levels for better verbosity control. Additionally, the code now employs modern f-string formatting for clearer and more concise log messages. Changes
TipsChat with CodeRabbit Bot (
|
I don't agree with this. I think that the folder needs to be shown in the js, where we hide and show different settings depending on what client it is. If they don't want it in a folder either they should empty that field, or we need to add an option to set whether they want to use labeler or our path setting . This was a new feature, we weren't able to send a path in the past. I'm curious if there is an incomplete_path option for qBitTorrent API also, it would be nice to be able to pass that. |
Need to change this line: sickchill/sickchill/gui/slick/js/core.js Line 1931 in fa39f38
--- $('#torrent_path_option').hide();
+++ $('#torrent_path_option').show();
+++ $('#torrent_path_option').find('.fileBrowser').show(); What I'm also curious about is if qBitTorrent xmlrpc can accept a |
Yes. That's a more complete solution giving the options. The new other torrent items can probably do this too. |
Add: https://qbittorrent-api.readthedocs.io/en/latest/apidoc/torrents.html |
clean up too many info logs for DownloadStation, converted to debug. |
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.
add this debug logging to others?
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.
Sure, this was added though when their API changes between dsm5 and 6 I think though. Up to you.
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.
I meant add logging.debug
for uT and rT etc. so there is some data if any of them aren't working.
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.
By all means, if you think it might be helpful at another time for debugging go for it.
@coderabbitai review |
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/gui/slick/js/core.js (1 hunks)
- sickchill/oldbeard/clients/download_station.py (3 hunks)
- sickchill/oldbeard/clients/qbittorrent.py (2 hunks)
Files skipped from review due to trivial changes (1)
- sickchill/oldbeard/clients/download_station.py
Additional comments: 3
sickchill/gui/slick/js/core.js (1)
- 1952-1954: The changes correctly implement the visibility of the torrent path options and file browser for the qBittorrent client in the SickChill UI. This should improve user experience by allowing users to set paths directly through the interface. Ensure that the corresponding elements are properly defined in the HTML and that any necessary event handlers are attached to make the file browser functional.
sickchill/oldbeard/clients/qbittorrent.py (2)
6-6: The import statement has been updated to include
logger
from thesickchill
settings. This is a necessary change for the new logging statements added in this file. Ensure that thelogger
is being used appropriately in all the places where logging is required.52-58: New logging statements have been added to the
_add_torrent_uri
and_add_torrent_file
methods using thelogger.debug
level. This is a good practice for providing more detailed information during the torrent addition process, which can be useful for troubleshooting. Ensure that the log messages are clear and provide enough context to be useful when debugging.
@staticmethod | ||
def __torrent_args(result: "TorrentSearchResult"): | ||
return dict( | ||
save_path=settings.TORRENT_DIR or None, | ||
save_path=settings.TORRENT_PATH or None, | ||
download_path=settings.TORRENT_PATH_INCOMPLETE or None, | ||
category=(settings.TORRENT_LABEL, settings.TORRENT_LABEL_ANIME)[result.show.is_anime] or settings.TORRENT_LABEL, | ||
is_paused=settings.TORRENT_PAUSED, | ||
) |
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 __torrent_args
method has been updated to use settings.TORRENT_PATH
and settings.TORRENT_PATH_INCOMPLETE
for the save_path
and download_path
parameters, respectively. This change is crucial for ensuring that the correct paths are used when adding torrents via qBittorrent. However, the use of a tuple to determine the category (settings.TORRENT_LABEL, settings.TORRENT_LABEL_ANIME)[result.show.is_anime]
assumes that result.show.is_anime
is a boolean that can be used as an index. This could lead to an IndexError
if is_anime
is not strictly a boolean. It would be safer to use a conditional expression instead.
- category=(settings.TORRENT_LABEL, settings.TORRENT_LABEL_ANIME)[result.show.is_anime] or settings.TORRENT_LABEL,
+ category=settings.TORRENT_LABEL_ANIME if result.show.is_anime else settings.TORRENT_LABEL,
Commitable suggestion
[!IMPORTANT]
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
@staticmethod | |
def __torrent_args(result: "TorrentSearchResult"): | |
return dict( | |
save_path=settings.TORRENT_DIR or None, | |
save_path=settings.TORRENT_PATH or None, | |
download_path=settings.TORRENT_PATH_INCOMPLETE or None, | |
category=(settings.TORRENT_LABEL, settings.TORRENT_LABEL_ANIME)[result.show.is_anime] or settings.TORRENT_LABEL, | |
is_paused=settings.TORRENT_PAUSED, | |
) | |
@staticmethod | |
def __torrent_args(result: "TorrentSearchResult"): | |
return dict( | |
save_path=settings.TORRENT_PATH or None, | |
download_path=settings.TORRENT_PATH_INCOMPLETE or None, | |
category=settings.TORRENT_LABEL_ANIME if result.show.is_anime else settings.TORRENT_LABEL, | |
is_paused=settings.TORRENT_PAUSED, | |
) |
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/download_station.py (3 hunks)
Files skipped from review due to trivial changes (1)
- sickchill/oldbeard/clients/download_station.py
@coderabbitai pause |
…ient Needs core.js to show these settings.
|
||
def _set_torrent_priority(self, result: "TorrentSearchResult"): | ||
if not self.api.app_preferences().get("queueing_enabled"): | ||
return True | ||
return (self.api.torrents_decrease_priority, self.api.torrents_increase_priority)[result.priority == 1](result.hash.lower) | ||
return (self.api.torrents_decrease_priority, self.api.torrents_increase_priority)[result.priority == 1](result.hash.lower()) |
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.
These missing parenthesis are a really old bug o can't believe I missed this all those years.
Fixes a QBT folder error where BlackHole path passed to QBT rather than using QBT defined folders.
https://discord.com/channels/502612977271439372/1173806065427697775
Summary by CodeRabbit
New Features
Refactor
info
todebug
level and adoption of f-string formatting.Bug Fixes