-
-
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
Emby notification : upgrade library_update to latests api endpoints #8736
Conversation
WalkthroughThis update introduces a method to fetch the path of a show within the Emby library, enhancing the 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 (1)
- sickchill/oldbeard/notifiers/emby.py (2 hunks)
Additional comments: 1
sickchill/oldbeard/notifiers/emby.py (1)
- 81-93: The modifications to the
update_library
method to support show-specific updates or fallback to a full library scan based on the show's path are well-implemented. However, there are a couple of points to consider:
- Error Handling: Similar to the
_get_show_path
method, consider catching more specific exceptions related to HTTP requests to avoid masking unrelated issues.- Logging and Debugging: The method logs the HTTP status and response content in case of a successful update. This is good for debugging purposes. However, consider adding more detailed logging in case of an exception to help diagnose issues more effectively.
Overall, the changes align with the PR objectives and should enhance the efficiency of the Emby notification system by avoiding unnecessary full library scans.
88f54a9
to
359d7a6
Compare
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/notifiers/emby.py (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- sickchill/oldbeard/notifiers/emby.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/notifiers/emby.py (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- sickchill/oldbeard/notifiers/emby.py
Propose a different solution without getting folder from Emby and using show.location this simplifies code as path is already known so don't need to look for it. SC log string.
Emby log
As you can see Emby uses ffprobe and logs new files. |
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/notifiers/emby.py (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- sickchill/oldbeard/notifiers/emby.py
part 2 simplification - still works.
|
@BKSteve that works only if the path is the same for SC and emby, in my case for example SC is in a docker image so it won't work we need things to work in all cases , for example if SC is on windows and Emby on a linux how will this work ? |
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/notifiers/emby.py (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- sickchill/oldbeard/notifiers/emby.py
acknowledged. |
7aa09ec
to
d988bdf
Compare
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/notifiers/emby.py (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- sickchill/oldbeard/notifiers/emby.py
Using our location won't be universal, as the emby server might be on a different location than SC. For emby the mount could be different, we could be using a UNC path or share in SC or emby and not the other, the name of the mount could be different, etc. |
use endpoint
/media/updated
with path param to refresh only needed show instead of full library scanpath is found by calling
/items
endpoint with indexerid (tvdbid in this case) and limiting results to only Seriesif we don't have exactly one result we fallback to a full library scan, this might happen for a new show added to SC but still not known to Emby for example
Summary by CodeRabbit