Add support for RPDB posters & Support fallback posters via MediaFusion posters #284
Conversation
WalkthroughThe changes encompass significant updates to the catalog retrieval and caching mechanisms, the introduction of a new configuration class, enhancements to user interface elements for RPDB integration, and the addition of functionality for managing movie poster URLs from the Rating Poster Database. Key functions have been modified or added to improve data parsing, caching strategies, and metadata handling, while new HTML and JavaScript elements facilitate user configuration for RPDB services. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend
participant Backend
participant RPDB
User->>Frontend: Interacts with RPDB configuration
Frontend->>Backend: Sends RPDB API key
Backend->>RPDB: Validates API key
RPDB-->>Backend: Returns validation result
Backend-->>Frontend: Confirms configuration
Frontend-->>User: Displays success message
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (3)
scrapers/rpdb.py (2)
15-25: LGTM with a minor suggestion!The function is implemented correctly and handles errors appropriately. Consider explicitly catching
httpx.TimeoutExceptionand logging a specific message for timeouts to differentiate it from other HTTP errors.except httpx.HTTPError as exc: logging.error(f"HTTP Exception for {exc.request.url} - {exc}") return False + except httpx.TimeoutException as exc: + logging.error(f"Timeout for {rpdb_poster_url} - {exc}") + return False except Exception as exc: logging.error(f"Exception for {rpdb_poster_url} - {exc}") return False
28-60: Looks good with a couple of suggestions!The function implements a caching mechanism correctly to avoid unnecessary requests. Consider the following improvements:
Use a constant for the Redis key prefix to avoid duplication and improve maintainability.
Use a more specific exception, such as
redis.exceptions.RedisError, for the Redis operations to differentiate from other exceptions.Define a constant for the Redis key prefix:
REDIS_KEY_PREFIX = "rpdb:" RPDB_SUPPORTED_SET = f"{REDIS_KEY_PREFIX}supported_ids" RPDB_UNSUPPORTED_HASH = f"{REDIS_KEY_PREFIX}unsupported_ids"
- Catch
redis.exceptions.RedisError:from redis.exceptions import RedisError ... try: if await REDIS_ASYNC_CLIENT.sismember(RPDB_SUPPORTED_SET, imdb_id): return rpdb_poster_url expiry_time = await REDIS_ASYNC_CLIENT.hget(RPDB_UNSUPPORTED_HASH, imdb_id) ... except RedisError as exc: logging.error(f"Redis error for {imdb_id} - {exc}") # Handle the error, e.g., return None or raise an exceptionresources/html/configure.html (1)
528-529: Remove empty<div>.The empty
<div>on lines 528-529 seems unnecessary. Please remove it to keep the markup clean and concise.Apply this change:
- <div class="mb-3">
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- api/main.py (6 hunks)
- db/crud.py (2 hunks)
- db/schemas.py (2 hunks)
- resources/html/configure.html (2 hunks)
- resources/js/config_script.js (5 hunks)
- scrapers/rpdb.py (1 hunks)
- utils/network.py (1 hunks)
Additional comments not posted (27)
scrapers/rpdb.py (3)
63-77: LGTM!The function correctly updates the poster URL for a single
MetaItembased on the user's RPDB config and catalog type. It properly delegates the poster update logic toupdate_single_rpdb_poster, maintaining a good separation of concerns.
80-95: Looks good!The function correctly updates the poster URLs for multiple
MetaItems concurrently usingasyncio.gather. It follows a similar pattern asupdate_rpdb_posterfor checking the user's RPDB config and catalog type, maintaining consistency. No major issues or improvements needed.
1-13: File-level code looks good!The imports are correctly organized and follow the standard convention. The constants for Redis keys and expiry are appropriately named and used consistently throughout the file. No issues or improvements needed at the file level.
db/schemas.py (2)
120-126: LGTM!The
RPDBConfigclass follows the existing pattern of configuration classes in the file. Theapi_keyfield is appropriately aliased for consistency, and the class is configured to ignore extra fields and populate by name, which is a good practice for configuration classes.
197-197: LGTM!The addition of the optional
rpdb_configfield to theUserDataclass enhances its structure by allowing it to hold configuration data specific to the RPDB service, thereby extending its functionality and flexibility in handling user data. The field is correctly typed, aliased for consistency, and defaults toNone, which is appropriate for a configuration field.api/main.py (14)
28-28: LGTM!The import statement is correct and necessary for the changes made in this file.
265-265: LGTM!The function call is correct and helps to streamline the parsing logic.
266-268: LGTM!The function call is correct and enhances the caching mechanism by determining the cache key based on the catalog type, catalog ID, skip, genre, and user data. It also identifies if the catalog is a watchlist.
270-275: LGTM!The code changes are correct and improve the flow of data retrieval by utilizing the cached data when available. The cached data is processed through
update_rpdb_posters, ensuring that the user data and catalog type are considered.
276-277: LGTM!The code changes are correct and ensure that the response headers are set appropriately based on the availability of the cache key.
279-281: LGTM!The function call is correct and handles the retrieval of metadata when no cache is available. It uses the catalog type, catalog ID, genre, skip, user data, request, and watchlist catalog flag to fetch the appropriate metadata.
283-288: LGTM!The code changes are correct and ensure that the metadata is cached for future retrieval, improving performance. The metadata is cached using the cache key, if available, with a TTL specified by
settings.meta_cache_ttl.
290-290: LGTM!The code changes are correct and ensure that the metadata is updated with the latest poster information before returning it to the user. The metadata is processed through
update_rpdb_posters, considering the user data and catalog type.
293-298: LGTM!The function is implemented correctly and streamlines the parsing of
genreandskipparameters. It handles the case whengenrecontains a&character, indicating thatskipis also provided. It extracts theskipvalue and converts it to an integer, defaulting to 0 if not provided or invalid.
301-323: LGTM!The function is implemented correctly and enhances the caching mechanism by determining the appropriate cache key based on various factors. It identifies if the catalog is a watchlist based on the user's streaming provider and catalog ID. It sets the cache key to
Nonefor watchlist catalogs and events. For movie and series catalogs, it appends the user's nudity and certification filters to the cache key.
Line range hint
326-358: LGTM!The function is implemented correctly and handles the retrieval of metadata based on the catalog type and user data. It considers various factors such as the user's IP address and watchlist catalog flag. The insertion of the special "delete all" meta item for watchlist catalogs is a nice addition.
Tools
Ruff
263-263: Do not perform function call
Dependsin argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable(B008)
409-412: LGTM!The code changes are correct and ensure that the search results are processed and updated with the latest poster information before returning them to the user. The search query is processed using the
crud.process_search_queryfunction based on the search query, catalog type, and user data. The resulting metadata is then updated with the latest poster information usingupdate_rpdb_posters, considering the user data and catalog type.
445-448: LGTM!The code changes are correct and handle the retrieval of cached metadata, ensuring that it is validated and updated with the latest poster information before returning it to the user. The cached metadata is retrieved using the cache key and validated using the
MetaItemmodel. If the validated metadata is empty, it raises a 404 error indicating that the meta ID was not found. The validated metadata is then updated with the latest poster information usingupdate_rpdb_poster, considering the user data and catalog type. The error handling for empty metadata is also appropriate.
471-473: LGTM!The code changes are correct and ensure that the metadata is validated and updated with the latest poster information before returning it to the user. The metadata is validated using the
MetaItemmodel and then updated with the latest poster information usingupdate_rpdb_poster, considering the user data and catalog type.resources/js/config_script.js (4)
320-326: LGTM!The code segment correctly introduces the
rpdbConfigobject to store the RPDB API key. The object creation is properly conditioned on the state of theenable_rpdbcheckbox, and the API key is validated to ensure it's not an empty string.
378-378: LGTM!The addition of
rpdbConfigto the data returned bygetUserDatais consistent with the introduction of therpdbConfigobject earlier in the code.
468-471: LGTM!The event listener correctly toggles the display of the
rpdb_configsection based on the state of theenable_rpdbcheckbox. ThesetElementDisplayfunction is used appropriately to show or hide the section.
561-561: LGTM!The password toggle setup for the
rpdb_api_keyinput field is correctly implemented using thesetupPasswordTogglefunction. This change is consistent with the password toggle setup for other sensitive input fields in the code.db/crud.py (1)
464-465: LGTM!The changes to the
get_series_metafunction improve the handling of theposterfield for series metadata. By introducing theposter_pathvariable and using it directly in the final output, the code becomes more readable and maintainable. The removal of theposterfield from the aggregation stages streamlines the data processing by eliminating unnecessary steps.Also applies to: 600-600
resources/html/configure.html (3)
480-484: LGTM!Adding a link to the MediaFlow setup guide is helpful for users to configure MediaFlow correctly. The code changes look good.
527-562: Looks good!The new RPDB configuration section is implemented correctly:
- The checkbox, description, and links provide clear context about the optional RPDB service for enhanced posters.
- The API key input is appropriately password-protected.
- The code is structured well and matches the existing conventions.
Great work on adding this optional feature!
545-547: LGTM!The description of the RPDB service is clear and helpful. It provides good context about how RPDB enhances the default MediaFusion poster behavior. The text looks good.
| @@ -235,7 +235,7 @@ def get_request_namespace(request: Request) -> str: | |||
| return namespace | |||
|
|
|||
|
|
|||
| def get_user_data(request: Request) -> UserData: | |||
| def get_user_data(request: Request, secret_str: str | None = None) -> UserData: | |||
There was a problem hiding this comment.
Tip
Codebase Verification
Remove the unused secret_str parameter from get_user_data.
The secret_str parameter is not used within the get_user_data function, nor is it passed in any function calls throughout the codebase. Removing this unused parameter will simplify the function signature and avoid confusion.
- File:
utils/network.py- Line: 238
-def get_user_data(request: Request, secret_str: str | None = None) -> UserData:
+def get_user_data(request: Request) -> UserData:
return request.userAnalysis chain
Verify the purpose and usage of the secret_str parameter.
I noticed that the function signature has been updated to accept an optional secret_str parameter, but the function body is not using this parameter. It's unclear why this parameter was added.
If the secret_str parameter is not needed, consider removing it to keep the function signature clean and avoid confusion.
To verify the purpose and usage of the secret_str parameter, run the following script:
If the secret_str parameter is not being used anywhere, consider applying this diff to remove it:
-def get_user_data(request: Request, secret_str: str | None = None) -> UserData:
+def get_user_data(request: Request) -> UserData:
return request.userScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for the usage of the `secret_str` parameter.
# Test 1: Search for the usage of the `secret_str` parameter in the `get_user_data` function.
# Expect: No occurrences of `secret_str` within the function body.
ast-grep --lang python --pattern $'def get_user_data($_, secret_str):
$$$
secret_str
$$$
'
# Test 2: Search for the invocations of the `get_user_data` function with the `secret_str` argument.
# Expect: Occurrences of `get_user_data` calls with the `secret_str` argument if the parameter is being used.
rg --type python -A 5 $'get_user_data\([^)]*secret_str'
Length of output: 185
Script:
#!/bin/bash
# Description: Search for the usage of the `secret_str` parameter.
# Test 1: Search for the usage of the `secret_str` parameter in the `get_user_data` function.
# Expect: No occurrences of `secret_str` within the function body.
ast-grep --lang python --pattern $'def get_user_data($_, secret_str):
$$$
secret_str
$$$
'
# Test 2: Correct the rg command to search for invocations of the `get_user_data` function with the `secret_str` argument.
# Expect: Occurrences of `get_user_data` calls with the `secret_str` argument if the parameter is being used.
rg -t py -A 5 'get_user_data\([^)]*secret_str'
Length of output: 416
There was a problem hiding this comment.
This is needed for annotating the open API config.
There was a problem hiding this comment.
@mhdzumair Thank you for providing that clarification. I apologize for my incorrect suggestion earlier. You're absolutely right that the secret_str parameter is important for annotating the OpenAPI config, even though it's not directly used within the function body.
I retract my previous suggestion to remove the parameter. It's crucial to keep it for proper API documentation. Thank you for catching this and explaining its purpose.
🐰💭
(A thoughtful rabbit, considering the new information)
Learnings added
Learnt from: mhdzumair
PR: mhdzumair/MediaFusion#284
File: utils/network.py:238-238
Timestamp: 2024-09-12T06:35:58.934Z
Learning: The `secret_str` parameter in the `get_user_data` function in `utils/network.py` is needed for annotating the OpenAPI config, even if it's not used within the function body itself.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
User Interface
Performance Improvements