feat: integrate dependency injection with kink library #859
Conversation
|
Warning Rate limit exceeded@iPromKnight has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 9 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. WalkthroughThe pull request introduces several changes across multiple files, primarily focusing on the integration of a new dependency, Changes
Poem
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 (
|
- Added dependency injection using the kink library to manage API instances and service initialization. - Updated various modules to utilize dependency injection for better modularity and testability. - Refactored API initialization and validation logic to be more centralized and consistent. - Enhanced Trakt, Plex, Overseerr, Mdblist, and Listrr services to use injected dependencies. - Updated CLI and service modules to align with the new dependency injection approach. - Modified pyproject.toml to include kink as a dependency. # Conflicts: # src/program/db/db_functions.py # src/program/utils/cli.py
eb78522 to
8a8509a
Compare
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Outside diff range and nitpick comments (24)
src/program/apis/__init__.py (2)
1-7: Add__all__declaration to explicitly define public API.To improve module interface clarity and address the unused imports warning, consider adding an
__all__declaration at the top of the file.+__all__ = [ + 'ListrrAPI', 'ListrrAPIError', + 'TraktAPI', 'TraktAPIError', + 'PlexAPI', 'PlexAPIError', + 'OverseerrAPI', 'OverseerrAPIError', + 'MdblistAPI', 'MdblistAPIError', + 'bootstrap_apis' +] from .listrr_api import ListrrAPI, ListrrAPIError🧰 Tools
🪛 Ruff
1-1:
.listrr_api.ListrrAPIErrorimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
2-2:
.trakt_api.TraktAPIErrorimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
3-3:
.plex_api.PlexAPIErrorimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
4-4:
.overseerr_api.OverseerrAPIErrorimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
5-5:
.mdblist_api.MdblistAPIErrorimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
9-41: Consider implementing a more robust dependency injection pattern.The current implementation could benefit from a more structured DI approach:
- Use a service provider pattern to manage API lifecycle
- Consider implementing interface abstractions for better testability
- Add proper cleanup/disposal of API instances
Would you like me to provide an example implementation of a more robust DI pattern?
src/program/services/content/overseerr.py (3)
18-18: Consider adding runtime safety checks for the api attributeWhile initializing
self.apito None is appropriate for DI, consider adding runtime checks in methods that useself.apito prevent potential NullPointerException ifvalidate()hasn't been called or failed.Example safety check:
def run(self): """Fetch new media from `Overseerr`""" + if not self.initialized or self.api is None: + logger.error("Overseerr API not initialized. Please ensure validate() was called and succeeded.") + return if self.settings.use_webhook and self.run_once: return
32-32: Consider adding DI container validationWhile the DI implementation is correct, consider adding a check to verify that OverseerrAPI is registered in the DI container to provide a more specific error message.
Example implementation:
def validate(self) -> bool: if not self.settings.enabled: return False if self.settings.api_key == "" or len(self.settings.api_key) != 68: logger.error("Overseerr api key is not set.") return False try: + if OverseerrAPI not in di: + logger.error("OverseerrAPI not registered in dependency injection container") + return False self.api = di[OverseerrAPI]
Line range hint
38-40: Enhance connection error loggingConsider including the configured URL in the connection error log to aid in troubleshooting.
- logger.error("Overseerr URL is not reachable, or it timed out") + logger.error(f"Overseerr URL '{self.settings.url}' is not reachable, or it timed out")src/program/services/content/mdblist.py (1)
17-17: Consider adding safeguards against None accessWhile initializing
self.apias None is consistent with the DI pattern, consider adding runtime checks to prevent potential NoneType errors if methods are called before validate().def run(self) -> Generator[MediaItem, None, None]: + if self.api is None: + logger.error("Mdblist API not initialized. Ensure validate() is called first.") + return items_to_yield = []src/program/services/content/listrr.py (3)
Line range hint
16-20: Consider refactoring the initialization pattern.While the change to initialize
apias None is good for dependency injection, the early return in the constructor is a code smell that could make the object's state unclear. Consider throwing an exception or using a factory pattern instead.def __init__(self): self.key = "listrr" self.settings = settings_manager.settings.content.listrr self.api = None - self.initialized = self.validate() - if not self.initialized: - return - logger.success("Listrr initialized!") + if not self.validate(): + raise ValueError("Listrr initialization failed") + logger.success("Listrr initialized!")
Line range hint
44-52: Consider separating validation concerns.The
validatemethod is handling multiple responsibilities: settings validation, dependency injection, and API validation. Consider splitting these into separate methods for better maintainability and testing.+ def _validate_settings(self) -> bool: + """Validate Listrr settings configuration.""" + if not self.settings.enabled: + return False + # ... rest of the settings validation logic ... + + def _initialize_api(self) -> bool: + """Initialize and validate API connection.""" + try: + self.api = di[ListrrAPI] + response = self.api.validate() + if not response.is_ok: + logger.error( + f"Listrr ping failed - Status Code: {response.status_code}, Reason: {response.response.reason}", + ) + return response.is_ok + except Exception as e: + logger.error(f"Listrr ping exception: {e}") + return False + def validate(self) -> bool: - """Validate Listrr settings.""" - if not self.settings.enabled: - return False - # ... rest of the method ... - try: - self.api = di[ListrrAPI] - response = self.api.validate() - if not response.is_ok: - logger.error( - f"Listrr ping failed - Status Code: {response.status_code}, Reason: {response.response.reason}", - ) - return response.is_ok - except Exception as e: - logger.error(f"Listrr ping exception: {e}") - return False + """Validate settings and initialize API.""" + return self._validate_settings() and self._initialize_api()
Line range hint
54-67: Improve error handling in run method.The current error handling silently returns None on failure, which could lead to unexpected behavior. Consider:
- Using more specific exception types
- Yielding an empty list instead of None
- Adding logging for successful operations
def run(self) -> Generator[MediaItem, None, None]: """Fetch new media from `Listrr`""" try: movie_items = self.api.get_items_from_Listrr("Movies", self.settings.movie_lists) show_items = self.api.get_items_from_Listrr("Shows", self.settings.show_lists) - except Exception as e: + except (ConnectionError, ValueError, AttributeError) as e: logger.error(f"Failed to fetch items from Listrr: {e}") - return + yield [] + return imdb_ids = movie_items + show_items listrr_items = [MediaItem({"imdb_id": imdb_id, "requested_by": self.api.key}) for imdb_id in imdb_ids if imdb_id.startswith("tt")] logger.info(f"Fetched {len(listrr_items)} items from Listrr") + logger.debug(f"Successfully processed Listrr items") yield listrr_itemssrc/program/services/content/plex_watchlist.py (3)
30-30: Add type hint for dependency injectionAdd a type hint to make the code more maintainable and IDE-friendly.
- self.api = di[PlexAPI] + self.api: PlexAPI = di[PlexAPI]
30-34: Refine error handlingThe broad Exception catch could mask specific issues. Consider catching more specific exceptions.
try: self.api = di[PlexAPI] self.api.validate_account() - except Exception as e: - logger.error(f"Unable to authenticate Plex account: {e}") + except KeyError as e: + logger.error("PlexAPI not registered in dependency injection container") + return False + except ConnectionError as e: + logger.error(f"Connection error while authenticating Plex account: {e}") + return False + except Exception as e: + logger.error(f"Unexpected error while authenticating Plex account: {e}") return False
Line range hint
23-52: Consider splitting validation logicThe
validatemethod has multiple responsibilities and high cyclomatic complexity. Consider splitting it into smaller, focused methods:
_validate_settings()_validate_plex_account()_validate_rss_feeds()Would you like me to provide a refactored implementation?
src/program/apis/listrr_api.py (1)
Line range hint
33-71: Consider refactoring complex method for better maintainability.The
get_items_from_Listrrmethod is marked withnoqa: C901, PLR0912indicating high complexity. Consider breaking it down into smaller, focused functions:
- List validation logic
- Item fetching logic
- ID resolution logic (TMDB → IMDB)
Here's a suggested refactoring approach:
def _validate_list_id(self, list_id: str) -> bool: return bool(list_id and len(list_id) == 24) def _fetch_page_items(self, content_type: str, list_id: str, page: int) -> tuple[list, int]: url = f"api/List/{content_type}/{list_id}/ReleaseDate/Descending/{page}" response = self.request_handler.execute(HttpMethod.GET, url) return response.data.get("items", []), response.data.get("pages", 1) def _resolve_item_id(self, item: dict, content_type: str) -> str | None: if imdb_id := item.get("imDbId"): return imdb_id if content_type == "Movies" and (tmdb_id := item.get("tmDbId")): return self.trakt_api.get_imdbid_from_tmdb(tmdb_id) return None def get_items_from_Listrr(self, content_type: str, content_lists: list[str]) -> list[str]: unique_ids: set[str] = set() if not content_lists: return list(unique_ids) for list_id in content_lists: if not self._validate_list_id(list_id): continue page, total_pages = 1, 1 while page <= total_pages: try: items, total_pages = self._fetch_page_items(content_type, list_id, page) for item in items: if item_id := self._resolve_item_id(item, content_type): unique_ids.add(item_id) except HTTPError as e: if e.response.status_code in [400, 404, 429, 500]: break except Exception as e: logger.error(f"An error occurred: {e}") break page += 1 return list(unique_ids)src/program/services/indexers/trakt.py (4)
Line range hint
31-47: Add type hints and consider recursion depthThe
copy_itemsmethod handles complex nested structures but lacks complete type hints. Also, deep recursion for large shows could be problematic.Consider these improvements:
- def copy_items(self, itema: MediaItem, itemb: MediaItem): + def copy_items(self, itema: MediaItem, itemb: MediaItem) -> MediaItem: """Copy attributes from itema to itemb recursively. Args: itema: Source media item itemb: Target media item Returns: MediaItem: Updated target item """Consider implementing an iterative approach for large shows to prevent stack overflow:
def copy_items(self, itema: MediaItem, itemb: MediaItem) -> MediaItem: if not isinstance(itemb, (Movie, Show)): raise ValueError(f"Unexpected item type: {type(itemb)}") is_anime = itema.is_anime or itemb.is_anime if isinstance(itemb, Movie): self.copy_attributes(itema, itemb) itemb.set("is_anime", is_anime) return itemb # Handle Show type if isinstance(itemb, Show): season_map = {s.number: s for s in itemb.seasons} for source_season in itema.seasons: if target_season := season_map.get(source_season.number): episode_map = {e.number: e for e in target_season.episodes} for source_episode in source_season.episodes: if target_episode := episode_map.get(source_episode.number): self.copy_attributes(source_episode, target_episode) target_season.set("is_anime", is_anime) itemb.set("is_anime", is_anime) return itemb
Line range hint
91-95: Improve error handling specificity in should_submitThe generic exception handling could mask specific issues. Consider handling specific exceptions and providing more context in error messages.
try: interval = timedelta(seconds=settings.update_interval) return datetime.now() - item.indexed_at > interval - except Exception: - logger.error(f"Failed to parse date: {item.indexed_at} with format: {interval}") + except (ValueError, TypeError) as e: + logger.error(f"Failed to parse date for item '{item.title}' (ID: {item.imdb_id}). " + f"indexed_at: {item.indexed_at}, interval: {interval}. Error: {str(e)}") return False
Line range hint
108-114: Remove unnecessary empty lines at end of fileThere are multiple empty lines at the end of the file that should be removed.
Keep only one empty line at the end of the file.
Line range hint
98-107: Enhance method documentationThe
_add_seasons_to_showmethod would benefit from a more comprehensive docstring including parameter types and return value.def _add_seasons_to_show(self, show: Show, imdb_id: str): - """Add seasons to the given show using Trakt API.""" + """Add seasons to the given show using Trakt API. + + Args: + show: Show object to add seasons to + imdb_id: IMDb ID of the show + + Note: + This method modifies the show object in place by adding seasons and episodes. + Seasons with number 0 (specials) are skipped. + """src/program/apis/plex_api.py (1)
25-26: LGTM! Consider adding docstring for improved documentation.The constructor changes align well with dependency injection principles by removing the direct dependency on RSS URLs. The initialization is safe and follows best practices.
Add a docstring to document the parameters and their types:
def __init__(self, token: str, base_url: str): + """Initialize PlexAPI instance. + + Args: + token (str): Plex authentication token + base_url (str): Base URL for the Plex server + """ self.rss_urls: Optional[List[str]] = Nonesrc/program/apis/overseerr_api.py (1)
Line range hint
82-117: Consider adding specific error handling for DI resolution.The
get_imdb_idmethod uses the injectedtrakt_apibut doesn't specifically handle DI resolution failures. Consider adding explicit error handling:def get_imdb_id(self, data) -> str | None: """Get imdbId for item from overseerr""" + try: + trakt_api = self.trakt_api # Attempt to resolve DI + except Exception as e: + logger.error(f"Failed to resolve TraktAPI dependency: {str(e)}") + return None + if data.mediaType == "show": external_id = data.tvdbId data.mediaType = "tv"src/program/program.py (1)
71-72: Enhance service initialization documentation.The docstring should provide more context about the initialization process and its dependencies.
Consider expanding the documentation:
def initialize_services(self): - """Initialize all services.""" + """Initialize all services. + + This method should be called after initialize_apis() to ensure all required + API dependencies are available. It sets up both requesting services + (Overseerr, PlexWatchlist, etc.) and processing services (TraktIndexer, + Scraping, etc.). + + Raises: + RuntimeError: If APIs are not properly initialized. + """src/routers/secure/default.py (1)
117-117: Avoid passingtrakt_api_keyas a parameter tohandle_oauth_callbackSince
TraktAPIshould encapsulate its own API key from the settings, passingtrakt_api_keyas a parameter is redundant. Access the API key within theTraktAPIclass to streamline the interface and reduce potential errors.Apply this change:
- success = trakt_api.handle_oauth_callback(trakt_api_key, code) + success = trakt_api.handle_oauth_callback(code)And modify the
handle_oauth_callbackmethod inTraktAPIaccordingly to use the internally stored API key.src/program/db/db_functions.py (3)
Line range hint
41-54: Add type hints to function parameters inget_item_by_id.Consider adding type hints to the
item_typesandsessionparameters in theget_item_by_idfunction to enhance code clarity and facilitate static type checking.Apply this diff to update the function signature:
-def get_item_by_id(item_id: str, item_types = None, session = None): +def get_item_by_id(item_id: str, item_types: list = None, session: Session = None):🧰 Tools
🪛 Ruff
13-13: Redefinition of unused
root_dirfrom line 10Remove definition:
root_dir(F811)
Line range hint
56-61: Add type hints to function parameters inget_items_by_ids.The
get_items_by_idsfunction would benefit from specifying type hints for theids,item_types, andsessionparameters. This improves code maintainability and assists with static analysis.Apply this diff to update the function signature:
-def get_items_by_ids(ids: list, item_types = None, session = None): +def get_items_by_ids(ids: list[str], item_types: list = None, session: Session = None):🧰 Tools
🪛 Ruff
13-13: Redefinition of unused
root_dirfrom line 10Remove definition:
root_dir(F811)
Line range hint
116-150: Specify type hint forsessionparameter indelete_seasons_and_episodes.Adding a type hint for the
sessionparameter in thedelete_seasons_and_episodesfunction enhances code readability and ensures consistency across the codebase.Apply this diff to update the function signature:
-def delete_seasons_and_episodes(session, season_ids: list[str], batch_size: int = 30): +def delete_seasons_and_episodes(session: Session, season_ids: list[str], batch_size: int = 30):🧰 Tools
🪛 Ruff
13-13: Redefinition of unused
root_dirfrom line 10Remove definition:
root_dir(F811)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
pyproject.toml(1 hunks)src/program/apis/__init__.py(1 hunks)src/program/apis/listrr_api.py(2 hunks)src/program/apis/overseerr_api.py(2 hunks)src/program/apis/plex_api.py(2 hunks)src/program/apis/trakt_api.py(4 hunks)src/program/db/db_functions.py(1 hunks)src/program/program.py(3 hunks)src/program/services/content/listrr.py(3 hunks)src/program/services/content/mdblist.py(3 hunks)src/program/services/content/overseerr.py(3 hunks)src/program/services/content/plex_watchlist.py(3 hunks)src/program/services/content/trakt.py(2 hunks)src/program/services/indexers/trakt.py(2 hunks)src/program/services/updaters/plex.py(3 hunks)src/routers/secure/default.py(2 hunks)src/routers/secure/webhooks.py(2 hunks)
🧰 Additional context used
🪛 Ruff
src/program/apis/__init__.py
1-1: .listrr_api.ListrrAPIError imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
2-2: .trakt_api.TraktAPIError imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
3-3: .plex_api.PlexAPIError imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
4-4: .overseerr_api.OverseerrAPIError imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
5-5: .mdblist_api.MdblistAPIError imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
src/program/db/db_functions.py
13-13: Redefinition of unused root_dir from line 10
Remove definition: root_dir
(F811)
🔇 Additional comments (28)
src/program/apis/__init__.py (1)
9-41: Verify API usage across the codebase.
Let's verify how these APIs are used to ensure the new DI pattern won't cause breaking changes.
✅ Verification successful
Let me gather more information about the API usage patterns to ensure we have a complete picture.
DI implementation is consistent and safe across the codebase
The verification shows that the dependency injection pattern is already properly implemented:
- All API instances are correctly obtained through the DI container (
di[API]) in services and routers - No direct instantiation of APIs exists outside of the bootstrap code
- API imports are only used for type hints
- The initialization order in
bootstrap_apis()is correct, considering that some APIs depend on others (e.g., ListrrAPI and OverseerrAPI depend on TraktAPI)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check API usage patterns across the codebase
# Check direct API instantiation that might break with DI
echo "Checking direct API instantiations..."
rg -l "= (?:Listrr|Trakt|Plex|Overseerr|Mdblist)API\(" --type py
# Check existing error handling patterns
echo "Checking error handling patterns..."
rg -l "(?:Listrr|Trakt|Plex|Overseerr|Mdblist)APIError" --type py
# Check settings usage patterns
echo "Checking settings access patterns..."
rg -l "settings\.(?:updaters|content)\." --type py
Length of output: 1151
Script:
#!/bin/bash
# Check how these APIs are actually used in the services
echo "Checking API usage in services..."
rg -A 3 "(?:Listrr|Trakt|Plex|Overseerr|Mdblist)API" src/program/services/ --type py
# Check if there are any direct imports of these APIs in services
echo "Checking API imports in services..."
rg "^from.*import.*(?:Listrr|Trakt|Plex|Overseerr|Mdblist)API" src/program/services/ --type py
# Check for dependency injection container usage
echo "Checking DI container usage..."
rg "di\[.*API\]" --type py
Length of output: 6574
src/program/services/content/overseerr.py (2)
6-6: LGTM: Clean import addition for dependency injection
The kink import is properly placed with other third-party imports, aligning with the PR's objective to implement dependency injection.
32-32: Verify DI setup and dependency chain
Let's verify the DI setup and check for potential circular dependencies.
✅ Verification successful
Let me gather more information about the dependency chain and DI setup to make a conclusive verification.
DI setup and dependency chain verified successfully
The verification shows a clean dependency setup:
- OverseerrAPI is properly registered in the DI container through
bootstrap_apis() - The dependency chain is straightforward: Overseerr service -> OverseerrAPI -> TraktAPI
- No circular dependencies detected
- The DI usage in the service follows the established pattern
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check OverseerrAPI registration and its dependencies
echo "Checking OverseerrAPI registration and dependencies..."
# Look for OverseerrAPI registration in DI container setup
rg -l "di\[OverseerrAPI\]" --type py
# Check for potential circular dependencies by finding all files that import OverseerrAPI
echo "\nChecking for potential circular dependencies..."
rg -l "from.*overseerr_api import|import.*overseerr_api" --type py
# Look for OverseerrAPI class definition to understand its dependencies
echo "\nChecking OverseerrAPI dependencies..."
rg -l "class OverseerrAPI" --type py
Length of output: 639
Script:
#!/bin/bash
# Check the DI registration in apis/__init__.py
echo "Checking DI registration in apis/__init__.py:"
rg -A 5 "OverseerrAPI" src/program/apis/__init__.py
# Check the OverseerrAPI class implementation
echo "\nChecking OverseerrAPI implementation:"
rg -A 10 "class OverseerrAPI" src/program/apis/overseerr_api.py
# Check the usage in overseerr.py service
echo "\nChecking usage in overseerr service:"
rg -A 5 "OverseerrAPI" src/program/services/content/overseerr.py
Length of output: 2557
src/routers/secure/webhooks.py (2)
11-11: LGTM: Clean import addition
The kink dependency import is correctly placed and necessary for the DI implementation.
54-54: Verify TraktAPI registration and configuration
The previous implementation used rate_limit=False. We need to ensure the DI-provided instance is configured correctly.
Let's verify the TraktAPI registration:
✅ Verification successful
Rate limiting is properly configured in TraktAPI initialization
The verification shows that the DI-provided TraktAPI instance is correctly configured with rate limiting. Looking at the implementation:
- The TraktAPI class now has built-in rate limiting through
get_rate_limit_params(max_calls=1000, period=300) - The DI registration in
__setup_trakt()uses the default constructor which includes this rate limiting - The old implementation with
rate_limit=Falsehas been removed in the git history
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check TraktAPI registration and configuration in the DI container
# Test 1: Find where TraktAPI is registered in the DI container
rg -l "di.register.*TraktAPI"
# Test 2: Check if rate_limit parameter is configured
ast-grep --pattern 'di.register\(\s*TraktAPI.*rate_limit\s*=\s*False'
Length of output: 103
Script:
#!/bin/bash
# Let's try a broader search to find TraktAPI registration and configuration
# Find files containing TraktAPI
rg -l "TraktAPI"
# Look for TraktAPI class definition and initialization
ast-grep --pattern 'class TraktAPI'
# Look for any rate_limit configuration
rg -A 3 "rate_limit.*=.*False"
# Look for di registration patterns more broadly
rg -A 3 "di\.register"
Length of output: 374
Script:
#!/bin/bash
# Let's examine the TraktAPI implementation and its usage
# Check TraktAPI implementation
cat src/program/apis/trakt_api.py
# Look for di registration in the main initialization files
cat src/program/apis/__init__.py
# Check the previous implementation in git history
git log -p src/routers/secure/webhooks.py | grep -A 5 "trakt_api"
Length of output: 18852
src/program/services/content/mdblist.py (2)
5-5: LGTM: Clean dependency injection import
The kink dependency is correctly imported for dependency injection support.
33-33: Verify API registration in DI container
Let's verify that MdblistAPI is properly registered in the DI container during application bootstrap.
✅ Verification successful
MdblistAPI registration is properly implemented
The verification shows that MdblistAPI is correctly registered in the DI container. Found in src/program/apis/__init__.py:
di[MdblistAPI] = mdblistApiThis confirms that the API registration is properly handled during application bootstrap, making it available for injection in src/program/services/content/mdblist.py.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for MdblistAPI registration in bootstrap/initialization code
rg -l "MdblistAPI.*di\[" --type py
Length of output: 36
Script:
#!/bin/bash
# Let's try different search patterns to find MdblistAPI registration
# First, search for MdblistAPI class definition
rg "class MdblistAPI" --type py -A 5
# Then search for any imports of MdblistAPI
rg "from.*MdblistAPI|import.*MdblistAPI" --type py
# Finally search for any DI registration patterns
rg "di\[.*\].*=.*MdblistAPI" --type py
Length of output: 1164
Script:
#!/bin/bash
# Let's search for DI registration patterns in initialization/bootstrap files
# First, find potential initialization files
fd "init|config|bootstrap|setup|main|app" --type f --extension py
# Then search for any DI container setup or registration patterns
rg "di\[|di\.set|register|container|provider" --type py
Length of output: 10965
src/program/services/content/listrr.py (2)
3-3: LGTM: Clean import of dependency injection module.
The import of di from kink is properly placed and follows the project's import style.
44-44: Verify dependency injection setup.
Let's ensure that ListrrAPI is properly registered with the DI container and that the pattern is consistently used.
✅ Verification successful
Let me check the DI registration by looking at the content of src/program/apis/__init__.py:
Dependency injection is properly implemented
The verification shows that:
- ListrrAPI is correctly registered in the DI container within
src/program/apis/__init__.py - The registration is conditional based on the feature being enabled
- The DI pattern is consistently used across related services
- There are no unexpected direct instantiations outside the DI registration
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for proper DI registration and usage
# Look for ListrrAPI registration with the DI container
echo "Checking for ListrrAPI registration..."
rg -l "di\[ListrrAPI\]" --type py
# Look for similar DI patterns in related files
echo "Checking for consistent DI usage..."
rg -l "from kink import di" --type py
# Check for potential direct instantiations that should use DI
echo "Checking for potential direct instantiations..."
rg "ListrrAPI\(" --type py
Length of output: 988
Script:
#!/bin/bash
# Check the DI registration in __init__.py
echo "Checking ListrrAPI registration in __init__.py..."
rg -A 5 -B 5 "ListrrAPI" src/program/apis/__init__.py
# Check if ListrrAPI is properly imported
echo "Checking ListrrAPI imports..."
rg "^from.*ListrrAPI|^import.*ListrrAPI" --type py
Length of output: 995
src/program/services/content/plex_watchlist.py (2)
6-6: LGTM: Clean dependency injection implementation
The switch from direct instantiation to dependency injection improves testability and follows good DI practices.
Also applies to: 17-17
Line range hint 54-71: LGTM: Clean implementation with proper error handling
The run method effectively combines watchlist and RSS items, with good use of set operations for deduplication and proper error handling.
pyproject.toml (1)
43-43: Consider version constraints and library maturity.
While the addition of kink aligns with the PR objectives for implementing dependency injection, there are a few considerations:
- The caret version constraint
^0.8.1allows updates to0.8.x. Consider:- Pinning to exact version (
0.8.1) for better reproducibility - Updating to latest
0.8.2for bug fixes
- Pinning to exact version (
- The library is pre-1.0.0, indicating potential API instability. Ensure:
- Comprehensive test coverage for DI implementations
- Monitoring of upstream changes
✅ Verification successful
Let me generate additional scripts to check for any existing dependency injection patterns and kink usage in the codebase:
The previous results show container usage but in a different context (media containers, test containers). We need to verify if there's any existing DI implementation or preparation for it before making a final recommendation about the kink version.
Version constraint is appropriate for this integration
The current version constraint ^0.8.1 is actually suitable for this integration because:
- The PR shows a comprehensive DI implementation with multiple
from kink import diimports across the codebase, indicating a well-planned integration - The project uses Poetry for dependency management which handles version resolution effectively
- While kink is pre-1.0.0, the 0.8.x series has been stable with only patch updates (current 0.8.2 vs 0.8.1)
- The caret (^) constraint is appropriate here as it allows patch updates while preventing breaking changes
No changes needed to the version constraint.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify kink version compatibility and usage
# Test 1: Check if kink is used in test files
rg -l "import kink|from kink" -g "test_*.py"
# Test 2: Check for any existing DI implementations
rg "Injectable|inject|container" -g "*.py"
Length of output: 6809
Script:
#!/bin/bash
# Check for any DI-related patterns or kink usage in Python files
rg -l "Inject|Provider|Container|Dependency" -g "*.py"
# Check for any existing dependency management or configuration setup
rg -l "dependency|dependencies|di setup|injection" -g "*.py"
# Look for any recent changes related to DI
git log -p | rg -i "dependency|injection|kink"
Length of output: 2066
src/program/apis/listrr_api.py (2)
3-3: LGTM: Clean import addition for dependency injection.
The kink import is properly placed following Python import conventions.
28-28: Verify TraktAPI configuration in DI container.
The change to use dependency injection is good, but we need to ensure the TraktAPI is properly configured in the DI container, especially regarding the rate_limit parameter that was previously set to False.
Let's verify the TraktAPI configuration:
✅ Verification successful
Based on the search results, I can now provide a final response:
Rate limiting is properly configured in TraktAPI initialization
The TraktAPI is configured with appropriate rate limiting in its constructor:
- Rate limit is set to 1000 requests per 300 seconds (5 minutes) via
rate_limit_params = get_rate_limit_params(max_calls=1000, period=300) - The API also uses caching with a 24-hour expiration via
trakt_cache = get_cache_params("trakt", 86400) - Both rate limiting and caching are properly applied when creating the session:
create_service_session(rate_limit_params=rate_limit_params, use_cache=True, cache_params=trakt_cache)
The change from direct instantiation to DI is safe and maintains proper rate limiting configuration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check TraktAPI configuration in DI container setup
# Expected: Find where TraktAPI is registered with its configuration
# Look for TraktAPI registration in bootstrap or configuration files
rg -l "TraktAPI" | xargs rg -A 5 "di\[TraktAPI\]|register.*TraktAPI"
# Look for rate_limit configuration
rg -A 3 "rate_limit.*TraktAPI"
Length of output: 3683
Script:
#!/bin/bash
# Let's check the TraktAPI class definition and initialization
ast-grep --pattern 'class TraktAPI {
$$$
}'
# Also check for any rate_limit related configuration in settings
rg -A 5 "trakt:|rate_limit"
# Look for TraktAPI instantiation in the DI setup
rg -A 10 "__setup_trakt|TraktAPI\("
Length of output: 36708
src/program/services/indexers/trakt.py (1)
6-6: Verify TraktAPI registration in DI container
The switch to dependency injection looks good, but we should ensure that TraktAPI is properly registered in the DI container before it's used.
Also applies to: 22-22
✅ Verification successful
TraktAPI is properly registered in the DI container
The TraktAPI is correctly registered in the DI container through the bootstrap_apis() function in src/program/apis/__init__.py. Specifically, the __setup_trakt() function instantiates and registers TraktAPI using di[TraktAPI] = traktApi. The implementation is clean and follows dependency injection best practices.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if TraktAPI is registered in the DI container
# Expected: Find the registration of TraktAPI in the DI container setup
# Look for TraktAPI registration
rg -l "di\[TraktAPI\]" | grep -v "trakt.py"
# Look for di.register or similar registration patterns for TraktAPI
ast-grep --pattern 'di.register($_, TraktAPI)'
Length of output: 247
Script:
#!/bin/bash
# Let's check the content of program/apis/__init__.py which might contain the registration
cat src/program/apis/__init__.py
# Also check for any other potential registration patterns
ast-grep --pattern 'di.factory($_, TraktAPI)'
# Look for TraktAPI class definition and its registration
rg -A 5 "class TraktAPI"
Length of output: 2510
src/program/apis/plex_api.py (1)
46-52: Verify RSS URL management usage across the codebase.
Since the RSS URL management has been refactored, we should verify all callers are updated accordingly.
✅ Verification successful
RSS URL management changes are safely implemented
The verification shows that all RSS URL access patterns are consistent with the new implementation:
- Direct access to
rss_urlsattribute is properly encapsulated withinplex_api.py - The only caller of
set_rss_urls()andget_items_from_rss()isplex_watchlist.py, which uses them correctly - PlexAPI constructor calls don't pass RSS URLs (they're set later via
set_rss_urls()) - The new
clear_rss_urls()method provides a safe way to reset RSS state
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for direct access to rss_urls and RSS-related method calls
# Test 1: Check for direct access to rss_urls attribute
echo "Checking for direct access to rss_urls attribute..."
rg "\.rss_urls\s*[^=]*=" --type py
# Test 2: Check for RSS-related method calls
echo "Checking for RSS method usage patterns..."
rg "set_rss_urls|clear_rss_urls|get_items_from_rss" --type py -A 2
# Test 3: Check for potential missed RSS URL assignments in constructors
echo "Checking for potential constructor calls with RSS URLs..."
ast-grep --pattern 'PlexAPI($$$)'
Length of output: 1977
src/program/apis/overseerr_api.py (2)
6-6: LGTM: Clean dependency injection import.
The kink import is properly placed with other third-party imports.
30-30: Verify DI configuration for TraktAPI.
While the dependency injection implementation looks correct, there are a few important aspects to verify:
- Ensure TraktAPI is properly registered in the DI container with the correct rate_limit configuration
- Consider adding error handling for DI resolution failures
Let's verify the TraktAPI registration:
✅ Verification successful
Based on the search results, I can now provide a final response:
TraktAPI DI setup is properly configured with rate limiting
The TraktAPI is correctly registered in the DI container with appropriate rate limiting:
- TraktAPI is initialized in
src/program/apis/__init__.pywithdi[TraktAPI] = traktApi - The TraktAPI class itself has rate limiting configured with
rate_limit_params = get_rate_limit_params(max_calls=1000, period=300)in its initialization - The rate limiting configuration is consistent with Trakt's API guidelines
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check TraktAPI registration and configuration
echo "Checking TraktAPI registration and configuration..."
# Look for TraktAPI DI registration
rg -l "di\[TraktAPI\]" --type py
# Look for TraktAPI configuration/registration code
ast-grep --pattern 'di.register(TraktAPI, $$$)'
Length of output: 429
Script:
#!/bin/bash
# Let's check for TraktAPI initialization and configuration patterns
echo "Checking TraktAPI initialization patterns..."
# Look for TraktAPI class definition and initialization
ast-grep --pattern 'class TraktAPI'
# Look for rate_limit configuration
rg "rate_limit" -A 5 -B 5 --type py
# Check DI setup in main or config files
fd "main|config|di" --type py --exec rg -l "TraktAPI|register"
Length of output: 51828
Script:
#!/bin/bash
# Let's check for TraktAPI initialization and DI registration
echo "Checking TraktAPI initialization and DI setup..."
# Look for TraktAPI class definition and initialization
rg -A 10 "class TraktAPI" --type py
# Look for DI registration patterns
rg "di\[" -B 5 -A 5 --type py
# Check for dependency injection setup files
fd "di|container" --extension py --exec rg -l "register|TraktAPI"
Length of output: 13943
src/program/program.py (1)
22-22: LGTM: Import for dependency injection bootstrap.
The import is correctly placed and aligns with the PR's objective of implementing dependency injection.
src/program/services/updaters/plex.py (3)
10-10: Imported di from kink for dependency injection
The import statement correctly brings in the di container from the kink library, enabling dependency injection within the module.
24-24: Initialized self.api to None in the constructor
Setting self.api to None in the __init__ method defers the instantiation to the validate method, which aligns with the new dependency injection approach.
46-49: Instantiated self.api using the dependency injection container in validate
Retrieving PlexAPI from the di container within the validate method is appropriate. This ensures that the API instance is properly managed and initialized only when needed.
src/program/services/content/trakt.py (3)
6-6: Proper import for dependency injection
The import statement correctly imports di from the kink library, enabling dependency injection.
17-17: Injection of TraktAPI instance via dependency injection
Successfully retrieves the TraktAPI instance from the dependency injection container, enhancing modularity and testability.
174-174: Ensured method returns IMDb IDs
The addition of return imdb_ids ensures the _extract_imdb_ids_with_none_type method returns the list of IMDb IDs as intended.
src/routers/secure/default.py (2)
8-9: Imports added for dependency injection
The imports of di from kink and TraktAPI are necessary for implementing dependency injection using the kink library.
Line range hint 87-88: Definition of TraktOAuthInitiateResponse model
The TraktOAuthInitiateResponse Pydantic model correctly defines the auth_url field for the Trakt OAuth initiation response.
src/program/apis/trakt_api.py (1)
35-42: Verify all instantiations of TraktAPI are updated to the new constructor
The initialization of TraktAPI now requires oauth_client_id, oauth_client_secret, and oauth_redirect_uri. Ensure that all places where TraktAPI is instantiated have been updated to pass these parameters.
Run the following script to find all instantiations of TraktAPI:
Review the output to ensure all instances are updated to the new constructor signature.
- Updated TraktAPI to accept settings via TraktModel, enabling OAuth configuration. - Added OAuth flow methods to handle authorization and token exchange. - Integrated TraktOauthModel into TraktModel for structured OAuth settings. - Modified API bootstrap to pass settings to TraktAPI. - Ensured backward compatibility with existing settings structure.
Summary by CodeRabbit
Release Notes
New Features
kinkdependency to enhance API management through dependency injection.PlexAPI.TraktAPIwith new methods for authorization flow.Improvements
Bug Fixes
These updates enhance the overall functionality and reliability of the application, providing a more modular and maintainable codebase.