feat: Active ongoing episode releasing on the time of airing #942
Conversation
WalkthroughThis pull request introduces comprehensive updates across multiple modules in the program, focusing on enhancing API interactions, logging, and media item handling. The changes include adding a new TVMaze API client, improving date and timezone management, introducing a custom logging level, and refining the program's scheduling and release tracking mechanisms. The modifications aim to provide more robust and flexible media item tracking with improved error handling and logging capabilities. Changes
Possibly related PRs
Suggested reviewers
Poem
Finishing Touches
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: 7
🔭 Outside diff range comments (1)
src/program/apis/trakt_api.py (1)
Line range hint
290-291: Simplify nested if statements into a single conditionAt lines 290-291, the nested
ifstatements can be combined to improve clarity.Apply this diff:
- if item.type == "episode": - if isinstance(item, Episode): + if item.type == "episode" and isinstance(item, Episode):
🧹 Nitpick comments (11)
src/program/apis/tvmaze_api.py (2)
171-172: Catch specific exceptions instead ofExceptionCatching the broad
Exceptionclass can mask unexpected errors. It's better to catch specific exceptions that you anticipate.Consider updating the exception handling:
- except Exception as e: + except (ValueError, TypeError) as e:Replace
ValueError, TypeErrorwith the specific exceptions that might be raised.
182-184: Avoid catching broad exceptionsSimilar to earlier, catching the general
Exceptionat lines 182-184 can obscure issues. Catch specific exceptions to handle known error conditions.Update the exception handling code:
- except Exception as e: + except (HTTPError, ValueError) as e:Replace
HTTPError, ValueErrorwith applicable exceptions.src/program/apis/trakt_api.py (2)
362-395: Ensure timezone-awareness when parsing datesIn the
_get_formatted_datemethod, consider handling timezone information more carefully to avoid potential bugs due to naive datetime objects.Ensure that all datetime objects are timezone-aware and handle cases where the timezone might not be specified.
Line range hint
354-354: Remove unused loop variableaired_atThe loop control variable
aired_atis not used within the loop body at line 354. This can cause confusion.Consider removing the unused variable:
- for item, aired_at in items: + for item, _ in items:Or, if
aired_atis not needed at all:- for item, aired_at in items: + for item in items:src/program/program.py (3)
220-263: Ensure consistent timezone handlingIn the
_update_ongoingmethod, consider standardizing the use of timezone-aware datetime objects to prevent potential issues with time comparisons.Ensure that all datetime objects (
current_time,trakt_time) are timezone-aware and use the same timezone for accurate comparisons.
354-355: Remove unused variableaired_atThe variable
aired_atis not used within the loop at lines 354-355. This may cause confusion.Update the loop to remove the unused variable:
- for item, aired_at in items: + for item, _ in items:Or:
- for item, aired_at in items: + for item in items:🧰 Tools
🪛 Ruff (0.8.2)
354-354: Loop control variable
aired_atnot used within loop body(B007)
366-368: Clarify logging message when updating item statesAt lines 366-368, the logging message states that item states have been updated, but the counter may not reflect the actual number of updated items due to exceptions.
Consider updating the logging logic to accurately reflect the number of items whose states were successfully updated.
src/program/media/item.py (1)
668-673: Use consistent timezone handling inSeason.is_releasedSimilar to
MediaItem, theis_releasedproperty inSeasonshould handle timezone-aware datetimes consistently.Ensure that
current_timeandself.aired_atare both timezone-aware and in the same timezone.src/program/__init__.py (1)
1-1: Consider expanding the module docstringThe docstring could be more descriptive about the module's purpose and functionality, especially given the new logging features.
-"""Program module.""" +"""Program module providing core functionality and logging configuration. + +This module initializes custom logging levels and exports core program entities. +"""src/program/apis/__init__.py (1)
10-10: Consider grouping related API importsThe TVMaze API import should be grouped with other API imports at the top of the file.
-from program.apis.tvmaze_api import TVMazeAPI +from .tvmaze_api import TVMazeAPI, TVMazeAPIErrorsrc/program/settings/models.py (1)
Line range hint
191-191: Document migration path for deprecated ratelimit fieldsThe deprecation warnings should include guidance on the migration path for users.
- ratelimit: bool = Field(default=True, deprecated=deprecation_warning) + ratelimit: bool = Field( + default=True, + deprecated="This field has been deprecated and will be removed in v2.0.0. " + "Rate limiting is now handled automatically by the API client." + )This change should be applied to all three occurrences of the deprecated ratelimit field.
Also applies to: 201-201, 211-211
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/program/__init__.py(1 hunks)src/program/apis/__init__.py(2 hunks)src/program/apis/trakt_api.py(2 hunks)src/program/apis/tvmaze_api.py(1 hunks)src/program/media/item.py(4 hunks)src/program/program.py(3 hunks)src/program/settings/models.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
src/program/program.py
290-291: Use a single if statement instead of nested if statements
(SIM102)
354-354: Loop control variable aired_at not used within loop body
(B007)
src/program/apis/tvmaze_api.py
154-155: Use a single if statement instead of nested if statements
(SIM102)
165-167: Use a single if statement instead of nested if statements
(SIM102)
src/program/media/item.py
402-402: Redefinition of unused get_top_title from line 322
(F811)
411-411: Redefinition of unused get_top_imdb_id from line 331
(F811)
419-419: Redefinition of unused get_aliases from line 339
(F811)
428-428: Redefinition of unused __hash__ from line 348
(F811)
431-431: Redefinition of unused reset from line 351
(F811)
🔇 Additional comments (3)
src/program/apis/trakt_api.py (1)
404-405: Avoid potential off-by-one error when scheduling midnight update
When calculating the next midnight time, ensure that the calculation correctly handles edge cases, such as when the current time is exactly at midnight.
Run the following script to verify the correctness of the midnight scheduling logic:
This script will help ensure that the scheduling logic always sets the next midnight correctly.
src/program/media/item.py (1)
178-187:
Handle timezone-aware datetime objects
In the is_released property at lines 178-187, ensure that aired_at and now are both timezone-aware to prevent incorrect time comparisons.
Apply this diff to ensure consistent timezone handling:
- now = datetime.now(tz=self.aired_at.tzinfo)
+ now = datetime.now(ZoneInfo("UTC"))Alternatively, ensure both aired_at and now are in the same timezone.
Likely invalid or redundant comment.
src/program/settings/models.py (1)
Line range hint 191-191: Verify usage of deprecated ratelimit fields
Let's ensure the deprecated fields aren't used in new code.
Also applies to: 201-201, 211-211
✅ Verification successful
Let me analyze the usage patterns of ratelimit to determine if there are any deprecated field usages in new code.
No deprecated ratelimit field usage found
The analysis shows that all ratelimit usages across the scrapers are consistent and follow the current pattern:
- Used as a boolean flag to enable/disable rate limiting
- Properly validated as boolean in configuration checks
- Consistently used with
get_rate_limit_params()for session creation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for usage of ratelimit in API implementations
rg -l "\.ratelimit" --type py
# Check for any new code using ratelimit
git diff origin/main..HEAD | rg "\+.*ratelimit"
Length of output: 420
Script:
#!/bin/bash
# Check the actual usage context of ratelimit in the files
for file in src/program/services/scrapers/*.py; do
echo "=== $file ==="
rg "\.ratelimit" -B 2 -A 2 "$file"
done
# Check if there are any new ratelimit-related changes in the PR
git diff origin/main..HEAD | rg "ratelimit" -B 2 -A 2
Length of output: 6771
src/program/apis/tvmaze_api.py
Outdated
| elif air_time > current_time: | ||
| # If we haven't found any future episode yet, or this one airs sooner | ||
| if not next_episode or air_time < next_episode: |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Combine nested conditions into a single if statement
The nested if statements at lines 165-167 can be merged for clearer logic flow.
Apply this diff:
- elif air_time > current_time:
- if not next_episode or air_time < next_episode:
- next_episode = air_time
+ elif air_time > current_time and (not next_episode or air_time < next_episode):
+ next_episode = air_time📝 Committable 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| elif air_time > current_time: | |
| # If we haven't found any future episode yet, or this one airs sooner | |
| if not next_episode or air_time < next_episode: | |
| elif air_time > current_time and (not next_episode or air_time < next_episode): | |
| next_episode = air_time |
🧰 Tools
🪛 Ruff (0.8.2)
165-167: Use a single if statement instead of nested if statements
(SIM102)
| def __hash__(self): | ||
| return hash(self.id) | ||
|
|
There was a problem hiding this comment.
Address method redefinitions
There are multiple redefinitions of the __hash__ method at lines 348 and 428-430.
Ensure that the __hash__ method is only defined once in the inheritance hierarchy to prevent unexpected behavior.
Consider removing the duplicate method definition.
🧰 Tools
🪛 Ruff (0.8.2)
428-428: Redefinition of unused __hash__ from line 348
(F811)
| def reset(self): | ||
| """Reset item attributes.""" | ||
| if self.type == "show": | ||
| for season in self.seasons: | ||
| for episode in season.episodes: | ||
| episode._reset() | ||
| season._reset() | ||
| elif self.type == "season": | ||
| for episode in self.episodes: | ||
| episode._reset() | ||
| self._reset() | ||
| if self.title: | ||
| self.store_state(States.Indexed) | ||
| else: | ||
| self.store_state(States.Requested) | ||
|
|
There was a problem hiding this comment.
Address method redefinitions
The reset method is redefined at lines 351 and 431-446.
To avoid confusion and potential bugs, ensure that method definitions are not duplicated. Consolidate the method into a single definition or properly override it if necessary.
🧰 Tools
🪛 Ruff (0.8.2)
431-431: Redefinition of unused reset from line 351
(F811)
| # Add custom log levels | ||
| logger.level("RELEASE", no=35, color="<magenta>") |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for logger configuration
The logger configuration should be wrapped in a try-catch block to handle potential initialization failures gracefully.
# Add custom log levels
-logger.level("RELEASE", no=35, color="<magenta>")
+try:
+ logger.level("RELEASE", no=35, color="<magenta>")
+except Exception as e:
+ logger.error(f"Failed to configure RELEASE log level: {e}")📝 Committable 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Add custom log levels | |
| logger.level("RELEASE", no=35, color="<magenta>") | |
| # Add custom log levels | |
| try: | |
| logger.level("RELEASE", no=35, color="<magenta>") | |
| except Exception as e: | |
| logger.error(f"Failed to configure RELEASE log level: {e}") |
| @@ -173,6 +173,7 @@ class TraktModel(Updatable): | |||
| most_watched_period: str = "weekly" | |||
| most_watched_count: int = 10 | |||
| update_interval: int = 86400 | |||
| release_delay_minutes: int = Field(default=60, description="Number of minutes to delay the state change after release") | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for release_delay_minutes
The release delay should have reasonable minimum and maximum values to prevent configuration errors.
- release_delay_minutes: int = Field(default=60, description="Number of minutes to delay the state change after release")
+ release_delay_minutes: int = Field(
+ default=60,
+ description="Number of minutes to delay the state change after release",
+ ge=0,
+ le=1440, # Max 24 hours
+ )📝 Committable 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| release_delay_minutes: int = Field(default=60, description="Number of minutes to delay the state change after release") | |
| release_delay_minutes: int = Field( | |
| default=60, | |
| description="Number of minutes to delay the state change after release", | |
| ge=0, | |
| le=1440, # Max 24 hours | |
| ) |
| from program.program import Event, Program # noqa: F401 | ||
|
|
||
| # Add custom log levels | ||
| logger.level("RELEASE", no=35, color="<magenta>") |
There was a problem hiding this comment.
this belongs in logging.py, you dont need to change this file :)
| @@ -7,18 +7,19 @@ | |||
| from .overseerr_api import OverseerrAPI, OverseerrAPIError | |||
| from .plex_api import PlexAPI, PlexAPIError | |||
| from .trakt_api import TraktAPI, TraktAPIError | |||
|
|
|||
| from program.apis.tvmaze_api import TVMazeAPI | |||
There was a problem hiding this comment.
from .tvmaze_api import TVMazeAPI
Also be sure to make a custom exception for TVMaze as well. Follow the same concepts as the other api modules do in this api's dir
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
src/program/apis/tvmaze_api.py (1)
Line range hint
220-377: Consider breaking down the _update_ongoing method into smaller functions.The method is quite complex with multiple responsibilities. Consider extracting these into separate methods:
_clear_old_scheduled_releases()_fetch_item_airtime(item: MediaItem) -> Optional[datetime]_process_release_schedule(item: MediaItem, air_time: datetime)Also, the variable
items_found_todayis assigned but never used.Remove the unused variable:
- items_found_today = False # Track if we found any items for today todays_releases = [] # Track items and their release times🧰 Tools
🪛 Ruff (0.8.2)
154-155: Use a single
ifstatement instead of nestedifstatements(SIM102)
165-167: Use a single
ifstatement instead of nestedifstatements(SIM102)
🧹 Nitpick comments (5)
src/program/apis/tvmaze_api.py (3)
154-155: Simplify nested conditions.The nested conditions can be simplified to improve readability.
Apply these changes:
- if hasattr(episode, 'number') and hasattr(episode, 'season'): - if episode.season == season_number and episode.number == episode_number: + if (hasattr(episode, 'number') and hasattr(episode, 'season') and + episode.season == season_number and episode.number == episode_number): - elif air_time > current_time: - if not next_episode or air_time < next_episode: - next_episode = air_time + elif air_time > current_time and (not next_episode or air_time < next_episode): + next_episode = air_timeAlso applies to: 165-167
🧰 Tools
🪛 Ruff (0.8.2)
154-155: Use a single
ifstatement instead of nestedifstatements(SIM102)
Line range hint
379-397: Add retry mechanism for database operations.The
_update_item_statemethod handles database operations but doesn't implement any retry mechanism for transient failures.Consider implementing a retry decorator:
def retry_on_db_error(max_retries=3, delay=1): def decorator(func): def wrapper(*args, **kwargs): for attempt in range(max_retries): try: return func(*args, **kwargs) except Exception as e: if attempt == max_retries - 1: raise logger.warning(f"Database operation failed, retrying in {delay}s: {e}") time.sleep(delay) return wrapper return decorator🧰 Tools
🪛 Ruff (0.8.2)
154-155: Use a single
ifstatement instead of nestedifstatements(SIM102)
165-167: Use a single
ifstatement instead of nestedifstatements(SIM102)
Line range hint
401-417: Consider adding health checks for scheduled jobs.The scheduler implementation looks good, but it might benefit from health monitoring.
Consider:
- Adding job execution metrics
- Implementing failure notifications
- Adding a dead man's switch for critical jobs
Example implementation:
def monitor_job_health(func): def wrapper(*args, **kwargs): start_time = time.time() try: result = func(*args, **kwargs) execution_time = time.time() - start_time logger.info(f"Job {func.__name__} completed in {execution_time:.2f}s") return result except Exception as e: logger.error(f"Job {func.__name__} failed: {e}") # Implement notification mechanism here raise return wrapper🧰 Tools
🪛 Ruff (0.8.2)
154-155: Use a single
ifstatement instead of nestedifstatements(SIM102)
165-167: Use a single
ifstatement instead of nestedifstatements(SIM102)
src/program/program.py (2)
293-294: Simplify nested if statementsThe nested if statements can be combined for better readability.
- if item.type == "episode": - if isinstance(item, Episode): + if item.type == "episode" and isinstance(item, Episode):🧰 Tools
🪛 Ruff (0.8.2)
293-294: Use a single
ifstatement instead of nestedifstatements(SIM102)
350-350: Remove unused variableThe variable
items_found_todayis assigned but never used.- items_found_today = True🧰 Tools
🪛 Ruff (0.8.2)
350-350: Local variable
items_found_todayis assigned to but never usedRemove assignment to unused variable
items_found_today(F841)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/program/apis/tvmaze_api.py(1 hunks)src/program/program.py(3 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
src/program/program.py
293-294: Use a single if statement instead of nested if statements
(SIM102)
350-350: Local variable items_found_today is assigned to but never used
Remove assignment to unused variable items_found_today
(F841)
src/program/apis/tvmaze_api.py
154-155: Use a single if statement instead of nested if statements
(SIM102)
165-167: Use a single if statement instead of nested if statements
(SIM102)
🔇 Additional comments (3)
src/program/program.py (3)
Line range hint 6-67: LGTM: Well-structured initialization of scheduler and time-related components
The addition of time-related imports and scheduler initialization in the constructor provides a solid foundation for the episode release timing feature.
379-397: LGTM: Well-structured item state update handling
The method effectively handles:
- State updates for individual items
- Error handling with session rollback
- Cleanup of scheduled releases
401-418: Verify timezone handling for midnight scheduling
While the code uses astimezone() for current time, ensure that the midnight schedule consistently triggers at the correct local time across different timezones and daylight saving transitions.
✅ Verification successful
Let me check the specific timezone handling in these files to verify if there are any inconsistencies or potential issues.
Timezone handling is properly implemented
The code demonstrates robust timezone handling:
- Uses
astimezone()consistently to work with local time - Explicitly handles UTC conversion for Trakt API times using
ZoneInfo("UTC") - Preserves timezone information when scheduling midnight jobs
- Other scheduled jobs are interval-based and not affected by timezone changes
The midnight scheduling will work correctly across different timezones and DST transitions because:
- The current time is obtained with proper timezone info (
datetime.now().astimezone()) - The midnight calculation preserves the timezone when using
replace() - The scheduler uses 'cron' format which automatically adjusts for timezone changes
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for timezone-related code in the codebase
# Look for timezone-related configurations
rg -l "timezone|ZoneInfo|tzinfo" --type py
# Check for other scheduler jobs that might be affected by timezone
ast-grep --pattern 'scheduler.add_job($$$)'
Length of output: 290
Script:
#!/bin/bash
# Check timezone handling in program.py and related files
rg -A 5 -B 5 "timezone|ZoneInfo|tzinfo" src/program/program.py
# Check all scheduler job configurations
rg -A 5 "scheduler.add_job" src/program/program.py
Length of output: 3148
src/program/apis/tvmaze_api.py
Outdated
| if not show: | ||
| logger.debug(f"Could not find show for {imdb_id} / {show_name}") | ||
| return None | ||
|
|
||
| # Get next episode | ||
| try: | ||
| response = self.request_handler._request(HttpMethod.GET, f"{self.BASE_URL}/shows/{show.id}/episodes") | ||
| episodes = response.data if response.is_ok else None | ||
| except HTTPError as e: | ||
| if e.response.status_code == 404: | ||
| episodes = None | ||
| else: | ||
| raise | ||
|
|
||
| if not episodes: | ||
| return None | ||
|
|
||
| # Find the next episode that hasn't aired yet | ||
| current_time = datetime.now() | ||
| next_episode = None | ||
| target_episode_time = None | ||
|
|
||
| for episode in episodes: | ||
| try: | ||
| if not episode.airstamp: | ||
| continue | ||
|
|
||
| # First try to get air time using network timezone | ||
| air_time = None | ||
| if (hasattr(show, 'network') and show.network and | ||
| hasattr(show.network, 'country') and show.network.country and | ||
| hasattr(show.network.country, 'timezone') and show.network.country.timezone and | ||
| episode.airdate and episode.airtime): | ||
|
|
||
| # Combine airdate and airtime in network timezone | ||
| network_tz = ZoneInfo(show.network.country.timezone) | ||
| air_datetime = f"{episode.airdate}T{episode.airtime}" | ||
| try: | ||
| # Parse the time in network timezone | ||
| air_time = datetime.fromisoformat(air_datetime).replace(tzinfo=network_tz) | ||
| # Only log network time for the target episode | ||
| if (season_number is not None and episode_number is not None and | ||
| hasattr(episode, 'number') and hasattr(episode, 'season') and | ||
| episode.season == season_number and episode.number == episode_number): | ||
| logger.debug(f"Network airs show at {air_time} ({show.network.country.timezone})") | ||
| except Exception as e: | ||
| logger.error(f"Failed to parse network air time: {e}") | ||
| air_time = None | ||
|
|
||
| # Fallback to airstamp if needed | ||
| if not air_time and episode.airstamp: | ||
| try: | ||
| air_time = datetime.fromisoformat(episode.airstamp.replace('Z', '+00:00')) | ||
| if (season_number is not None and episode_number is not None and | ||
| hasattr(episode, 'number') and hasattr(episode, 'season') and | ||
| episode.season == season_number and episode.number == episode_number): | ||
| logger.debug(f"Using UTC airstamp: {air_time}") | ||
| except Exception as e: | ||
| logger.error(f"Failed to parse airstamp: {e}") | ||
| continue | ||
|
|
||
| if not air_time: | ||
| continue | ||
|
|
||
| # Convert to local time | ||
| air_time = air_time.astimezone(current_time.tzinfo) | ||
|
|
||
| # Check if this is the specific episode we want | ||
| if season_number is not None and episode_number is not None: | ||
| if hasattr(episode, 'number') and hasattr(episode, 'season'): | ||
| if episode.season == season_number and episode.number == episode_number: | ||
| # Found our target episode | ||
| if hasattr(episode, 'name'): | ||
| logger.debug(f"Found S{season_number}E{episode_number} '{episode.name}' airing at {air_time}") | ||
| else: | ||
| logger.debug(f"Found S{season_number}E{episode_number} airing at {air_time}") | ||
| target_episode_time = air_time | ||
| break # No need to continue looking | ||
|
|
||
| # If we're looking for next episode and this one is in the future | ||
| elif air_time > current_time: | ||
| # If we haven't found any future episode yet, or this one airs sooner | ||
| if not next_episode or air_time < next_episode: | ||
| next_episode = air_time | ||
|
|
||
| except Exception as e: | ||
| logger.error(f"Failed to process episode {getattr(episode, 'number', '?')}: {e}") | ||
| continue | ||
|
|
||
| # Return target episode time if we found one, otherwise return next episode | ||
| if target_episode_time is not None: | ||
| return target_episode_time | ||
|
|
||
| if next_episode: | ||
| logger.debug(f"Next episode airs at {next_episode}") | ||
| return next_episode | ||
|
|
||
| except Exception as e: | ||
| logger.error(f"Error fetching TVMaze data for {imdb_id}: {e}") | ||
| return None |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider breaking down the complex method into smaller, focused functions.
The get_show_by_imdb method handles multiple responsibilities including IMDB ID formatting, show lookup via different methods, and episode airtime processing. Consider extracting these into separate methods for better maintainability:
_format_imdb_id(imdb_id: str) -> str_lookup_show_by_name(show_name: str) -> Optional[Show]_lookup_show_by_imdb(imdb_id: str) -> Optional[Show]_get_show_episodes(show_id: int) -> Optional[List[Episode]]
Additionally, consider consolidating the error handling pattern to reduce code duplication:
def _make_api_request(self, endpoint: str, params: dict) -> Optional[Any]:
try:
response = self.request_handler._request(HttpMethod.GET, f"{self.BASE_URL}/{endpoint}", params=params)
return response.data if response.is_ok else None
except HTTPError as e:
if e.response.status_code == 404:
return None
raise🧰 Tools
🪛 Ruff (0.8.2)
154-155: Use a single if statement instead of nested if statements
(SIM102)
165-167: Use a single if statement instead of nested if statements
(SIM102)
src/program/program.py
Outdated
|
|
||
| tvmaze_api = di[TVMazeAPI] | ||
| # Get show title - for episodes, use the parent show's title | ||
| show_name = item.get_top_title() | ||
| if not show_name: | ||
| continue | ||
|
|
||
| # Skip if it's just an episode number | ||
| if show_name.lower().startswith("episode "): | ||
| continue | ||
|
|
||
| # For episodes, pass the season and episode numbers | ||
| season_number = None | ||
| episode_number = None | ||
| if item.type == "episode": | ||
| if isinstance(item, Episode): | ||
| season_number = item.parent.number if item.parent else None | ||
| episode_number = item.number | ||
|
|
||
| tvmaze_time = tvmaze_api.get_show_by_imdb( | ||
| item.imdb_id, | ||
| show_name=show_name, | ||
| season_number=season_number, | ||
| episode_number=episode_number | ||
| ) | ||
| if tvmaze_time: | ||
| logger.debug(f"TVMaze airtime for {item.log_string}: {tvmaze_time}") | ||
| except Exception as e: | ||
| logger.error(f"Failed to fetch airtime from TVMaze for {item.log_string}: {e}") | ||
|
|
||
| # Use the earliest available time | ||
| if trakt_time and tvmaze_time: | ||
| air_time = min(trakt_time, tvmaze_time) | ||
| logger.debug(f"Using earliest time between Trakt ({trakt_time}) and TVMaze ({tvmaze_time})") | ||
| else: | ||
| air_time = trakt_time or tvmaze_time | ||
| if not air_time: | ||
| logger.debug(f"No airtime available from either Trakt or TVMaze for {item.log_string}") | ||
| continue # Skip this item | ||
|
|
||
| if not air_time: # Add explicit check | ||
| logger.debug(f"No valid air time found for {item.log_string}") | ||
| continue | ||
|
|
||
| # Store the local time in the database | ||
| item.aired_at = air_time | ||
| session.merge(item) | ||
| logger.debug(f"Updated airtime for {item.log_string} to {air_time}") | ||
|
|
||
| # Calculate delayed release time | ||
| delay_minutes = settings_manager.settings.content.trakt.release_delay_minutes | ||
| delayed_time = air_time + timedelta(minutes=delay_minutes) | ||
|
|
||
| # Format times for display (e.g., "8:00 PM") | ||
| air_time_str = air_time.strftime("%I:%M %p").lstrip('0') | ||
| release_time_str = delayed_time.strftime("%I:%M %p").lstrip('0') | ||
|
|
||
| # If it aired in the past (including delay), release it immediately | ||
| if delayed_time <= current_time: | ||
| logger.log("PROGRAM", f"- {item.log_string} was scheduled to release at {air_time_str}") | ||
| # Trigger immediate state update | ||
| previous_state, new_state = item.store_state() | ||
| if previous_state != new_state: | ||
| self.em.add_event(Event(emitted_by="UpdateOngoing", item_id=item.id)) | ||
| logger.debug(f"Updated state for {item.log_string} ({item.id}) from {previous_state.name} to {new_state.name}") | ||
| continue | ||
|
|
||
| # For future items, only schedule if they air today | ||
| if (air_time.year == current_time.year and | ||
| air_time.month == current_time.month and | ||
| air_time.day == current_time.day): | ||
| items_found_today = True | ||
| todays_releases.append((item.log_string, release_time_str)) | ||
| logger.debug(f"Found today's item: {item.log_string}") | ||
| logger.log("PROGRAM", f"- {item.log_string} will release at {release_time_str} (included {delay_minutes}min delay)") | ||
| # Add to scheduled releases if not already there | ||
| if item.id not in self.scheduled_releases: | ||
| self.scheduled_releases[item.id] = delayed_time | ||
| except Exception as e: | ||
| logger.error(f"Failed to fetch airtime for {item.log_string}: {e}") | ||
| continue | ||
|
|
||
| # Commit any airtime updates | ||
| session.commit() | ||
|
|
||
| # Log summary of today's releases | ||
| if todays_releases: | ||
| logger.log("PROGRAM", "\nToday's releases:") | ||
| for item_name, release_time in sorted(todays_releases, key=lambda x: x[1]): | ||
| logger.log("PROGRAM", f" • {item_name} at {release_time}") | ||
| else: | ||
| logger.log("PROGRAM", "\nNo releases scheduled for today") | ||
|
|
||
| except Exception as e: | ||
| logger.error(f"Failed to update state for item with ID {item_id}: {e}") | ||
| session.rollback() | ||
| logger.error(f"Database error in _update_ongoing: {e}") | ||
| raise | ||
| except Exception as e: | ||
| logger.error(f"Error in _update_ongoing: {e}") |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider breaking down the _update_ongoing method into smaller functions
The method is handling multiple responsibilities including:
- Clearing old scheduled releases
- Fetching air times from multiple APIs
- Updating database records
- Scheduling future releases
Consider extracting these into separate methods for better maintainability and testability.
Here's a suggested structure:
def _update_ongoing(self) -> None:
try:
current_time = self._get_current_time()
self._clear_old_scheduled_releases(current_time)
self._process_unreleased_items(current_time)
except Exception as e:
logger.error(f"Error in _update_ongoing: {e}")
def _clear_old_scheduled_releases(self, current_time: datetime) -> None:
# Lines 229-233
def _process_unreleased_items(self, current_time: datetime) -> None:
# Lines 238-371
def _fetch_air_times(self, item: MediaItem) -> tuple[datetime, datetime]:
# Extract air time fetching logic🧰 Tools
🪛 Ruff (0.8.2)
293-294: Use a single if statement instead of nested if statements
(SIM102)
350-350: Local variable items_found_today is assigned to but never used
Remove assignment to unused variable items_found_today
(F841)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/program/program.py (1)
220-376: 🛠️ Refactor suggestionRefactor
_update_ongoingmethod into smaller functionsThe
_update_ongoingmethod currently handles multiple responsibilities:
- Clearing old scheduled releases
- Fetching airtimes from Trakt and TVMaze APIs
- Updating database records
- Scheduling and logging future releases
- Error handling
Splitting this method into smaller, cohesive functions enhances maintainability and testability.
Consider refactoring as follows:
def _update_ongoing(self) -> None: try: current_time = self._get_current_time() self._clear_old_scheduled_releases(current_time) items = self._fetch_items_to_update() self._process_items(items, current_time) except Exception as e: logger.error(f"Error in _update_ongoing: {e}") def _get_current_time(self) -> datetime: return datetime.now().astimezone() def _clear_old_scheduled_releases(self, current_time: datetime) -> None: # Logic from lines 229-233 def _fetch_items_to_update(self): # Logic from lines 239-245 return items def _process_items(self, items, current_time: datetime) -> None: # Logic from lines 247-370🧰 Tools
🪛 Ruff (0.8.2)
293-294: Use a single
ifstatement instead of nestedifstatements(SIM102)
350-350: Local variable
items_found_todayis assigned to but never usedRemove assignment to unused variable
items_found_today(F841)
🧹 Nitpick comments (2)
src/program/program.py (2)
293-297: Simplify nestedifstatements when assigning season and episode numbersCombining the nested
ifstatements into a single condition improves readability and reduces code complexity.Apply this diff to streamline the conditions:
-season_number = None -episode_number = None -if item.type == "episode": - if isinstance(item, Episode): - season_number = item.parent.number if item.parent else None - episode_number = item.number +if item.type == "episode" and isinstance(item, Episode): + season_number = item.parent.number if item.parent else None + episode_number = item.number +else: + season_number = None + episode_number = None🧰 Tools
🪛 Ruff (0.8.2)
293-294: Use a single
ifstatement instead of nestedifstatements(SIM102)
350-350: Remove unused variableitems_found_todayThe variable
items_found_todayis assigned but not used elsewhere in the_update_ongoingmethod. Removing it eliminates unnecessary code.Apply this diff to remove the unused variable:
- items_found_today = True # Track if we found any items for today🧰 Tools
🪛 Ruff (0.8.2)
350-350: Local variable
items_found_todayis assigned to but never usedRemove assignment to unused variable
items_found_today(F841)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/program/program.py(3 hunks)src/routers/secure/items.py(2 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
src/program/program.py
293-294: Use a single if statement instead of nested if statements
(SIM102)
350-350: Local variable items_found_today is assigned to but never used
Remove assignment to unused variable items_found_today
(F841)
|
|
||
| # Get all matching items and use the first one | ||
| items = session.execute(query).unique().scalars().all() | ||
| if not items: | ||
| raise HTTPException(status_code=404, detail="Item not found") | ||
|
|
||
| # Use the first item if there are multiple | ||
| item = items[0] | ||
| if len(items) > 1: | ||
| # Log details about each duplicate | ||
| logger.warning(f"Multiple items found for ID {id}:") | ||
| for i, dupe in enumerate(items): | ||
| logger.warning(f" {i+1}. {dupe.type} - {dupe.log_string} (ID: {dupe.id})") | ||
| logger.warning(f"Using first item: {item.type} - {item.log_string}") | ||
|
|
||
| return item.to_extended_dict(with_streams=False) |
There was a problem hiding this comment.
Handle duplicate items more robustly in get_item
Returning the first item when multiple items are found for a given ID may lead to inconsistent behavior. This could mask underlying data integrity issues.
Consider updating the logic to:
- Investigate why duplicates exist and address the root cause in the database.
- Modify the function to raise a
HTTPExceptionindicating multiple items found, prompting to specify a more precise query or to resolve duplicates.
Alternatively, if duplicates are acceptable in this context, clearly document the behavior and ensure it aligns with the intended application logic.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/program/program.py (1)
293-294: Address static analysis warnings
- Simplify nested if statements:
- if item.type == "episode": - if isinstance(item, Episode): + if item.type == "episode" and isinstance(item, Episode):
- Remove unused variable:
- items_found_today = False # Track if we found any items for todayAlso applies to: 350-350
🧰 Tools
🪛 Ruff (0.8.2)
293-294: Use a single
ifstatement instead of nestedifstatements(SIM102)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/program/program.py(3 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
src/program/program.py
293-294: Use a single if statement instead of nested if statements
(SIM102)
350-350: Local variable items_found_today is assigned to but never used
Remove assignment to unused variable items_found_today
(F841)
🔇 Additional comments (3)
src/program/program.py (3)
432-452: LGTM! Well-structured release processing
The method has good error handling, proper cleanup of scheduled releases, and appropriate logging of release events.
456-473: LGTM! Well-implemented scheduling logic
The scheduling implementation includes:
- Appropriate midnight scheduling for updates
- Good misfire grace period handling
- Immediate execution on startup
220-377: 🛠️ Refactor suggestion
Method is too complex and needs refactoring
The _update_ongoing method is handling multiple responsibilities including:
- Clearing old scheduled releases
- Fetching air times from multiple APIs
- Updating database records
- Scheduling future releases
Consider breaking it down into smaller, focused methods for better maintainability and testability.
def _update_ongoing(self) -> None:
+ try:
+ current_time = datetime.now().astimezone()
+ self._clear_old_scheduled_releases(current_time)
+ self._process_unreleased_items(current_time)
+ except Exception as e:
+ logger.error(f"Error in _update_ongoing: {e}")
+
+def _clear_old_scheduled_releases(self, current_time: datetime) -> None:
+ new_scheduled_releases = {}
+ for k, v in self.scheduled_releases.items():
+ if v > current_time:
+ new_scheduled_releases[k] = v
+ self.scheduled_releases = new_scheduled_releases
+
+def _process_unreleased_items(self, current_time: datetime) -> None:
# Rest of the current _update_ongoing logicLikely invalid or redundant comment.
🧰 Tools
🪛 Ruff (0.8.2)
293-294: Use a single if statement instead of nested if statements
(SIM102)
350-350: Local variable items_found_today is assigned to but never used
Remove assignment to unused variable items_found_today
(F841)
src/program/program.py
Outdated
| self.scheduled_releases = {} # Track scheduled releases | ||
| self.scheduler = BackgroundScheduler() # Initialize the scheduler | ||
| self.scheduler.start() # Start the scheduler |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider moving scheduler start to a more appropriate location
Starting the scheduler in __init__ could be problematic if object creation fails. Consider moving the scheduler start to the start() method where other initializations occur, and add proper error handling.
def __init__(self):
super().__init__(name="Riven")
self.initialized = False
self.running = False
self.services = {}
self.enable_trace = settings_manager.settings.tracemalloc
self.em = EventManager()
self.scheduled_releases = {} # Track scheduled releases
- self.scheduler = BackgroundScheduler() # Initialize the scheduler
- self.scheduler.start() # Start the scheduler
+ self.scheduler = BackgroundScheduler()Committable suggestion skipped: line range outside the PR's diff.
| if item_id in self.scheduled_releases and self.scheduled_releases[item_id] <= current_time: | ||
| del self.scheduled_releases[item_id] |
There was a problem hiding this comment.
💡 Codebase verification
Based on the analysis of the codebase, I can now provide the final response:
Race condition concern is valid - synchronization needed
The code shows that Program class inherits from threading.Thread and operates in a multi-threaded environment. The scheduled_releases dictionary is accessed from multiple contexts:
- Main thread in
_update_item_state - Background scheduler thread in
_process_release - Dictionary iteration and modification in multiple places
Implement the suggested lock-based synchronization to prevent potential race conditions.
🔗 Analysis chain
Potential race condition in scheduled releases cleanup
The check and removal of scheduled releases could lead to a race condition in a multi-threaded environment. Consider using a thread-safe operation or lock.
+ from threading import Lock
+
class Program(threading.Thread):
def __init__(self):
# ... existing init code ...
+ self._scheduled_releases_lock = Lock()
+
+ def _remove_scheduled_release(self, item_id: str, current_time: datetime) -> None:
+ with self._scheduled_releases_lock:
+ if item_id in self.scheduled_releases and self.scheduled_releases[item_id] <= current_time:
+ del self.scheduled_releases[item_id]Let me analyze the concurrent access patterns to scheduled_releases further.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for concurrent access to scheduled_releases
rg -A 5 "self\.scheduled_releases" | grep -v "# Track scheduled releases"
Length of output: 2996
Script:
#!/bin/bash
# Check if Program class inherits from Thread and look for any threading-related code
ast-grep --pattern 'class Program(threading.Thread)'
# Check for any other thread creation or async patterns
rg "Thread|threading|asyncio|concurrent" -A 3
Length of output: 16267
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/program/program.py (3)
250-250: Fix unused loop variableThe loop variable
aired_atis not used within the loop body.- for item, aired_at in items: + for item, _ in items:🧰 Tools
🪛 Ruff (0.8.2)
250-250: Loop control variable
aired_atnot used within loop bodyRename unused
aired_atto_aired_at(B007)
444-468: Consider using explicit transaction managementThe method could benefit from explicit transaction management to ensure atomicity of the release operation.
def _process_release(self, item_id: str, log_string: str) -> None: """Process a scheduled release at its designated time.""" try: with db.Session() as session: + with session.begin(): # Explicit transaction item = session.execute( select(MediaItem).where(MediaItem.id == item_id) ).unique().scalar_one_or_none() if item: if item.last_state in [States.Ongoing, States.Unreleased]: previous_state, new_state = item.store_state() if previous_state != new_state: self.em.add_event(Event("StateTransition", item_id=item_id)) release_time = datetime.now().astimezone().strftime("%I:%M %p").lstrip('0') logger.log("RELEASE", f"🎬 Released at {release_time}: {log_string}") session.merge(item) - session.commit() else: logger.error(f"Item {item_id} not found during scheduled release") # Clean up the scheduled release + with self._scheduled_releases_lock: self.scheduled_releases.pop(item_id, None)
474-474: Remove unused variableThe
midnightvariable is calculated but never used.- midnight = (current_time + timedelta(days=1)).replace(hour=0, minute=0, second=0, microsecond=0)🧰 Tools
🪛 Ruff (0.8.2)
474-474: Local variable
midnightis assigned to but never usedRemove assignment to unused variable
midnight(F841)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/program/program.py(6 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
src/program/program.py
250-250: Loop control variable aired_at not used within loop body
Rename unused aired_at to _aired_at
(B007)
474-474: Local variable midnight is assigned to but never used
Remove assignment to unused variable midnight
(F841)
🔇 Additional comments (4)
src/program/program.py (4)
605-611: LGTM: Clean shutdown implementation
The stop method properly handles the scheduler shutdown and program termination.
236-236:
Add thread safety for scheduled releases
The scheduled_releases dictionary is accessed from multiple threads (scheduler and main thread) without synchronization.
def __init__(self):
# ... existing init code ...
self.scheduled_releases = {} # Track scheduled releases
+ self._scheduled_releases_lock = threading.Lock()
def _update_ongoing(self) -> None:
try:
current_time = datetime.now().astimezone()
logger.log("PROGRAM", "Checking for upcoming releases...")
trakt_api = di[TraktAPI]
# Clear old scheduled releases
+ with self._scheduled_releases_lock:
self.scheduled_releases = {k: v for k, v in self.scheduled_releases.items()
if v > current_time}Likely invalid or redundant comment.
66-76: 🛠️ Refactor suggestion
Consider moving scheduler initialization to start() method
Starting the scheduler in __init__ could be problematic if object creation fails. Consider moving the scheduler initialization and start to the start() method where other initializations occur.
def __init__(self):
super().__init__(name="Riven")
self.initialized = False
self.running = False
self.services = {}
self.enable_trace = settings_manager.settings.tracemalloc
self.em = EventManager()
self.scheduled_releases = {} # Track scheduled releases
- self.scheduler = BackgroundScheduler(
- timezone=datetime.now().astimezone().tzinfo, # Use system timezone
- )
- logging.getLogger('apscheduler').setLevel(logging.WARNING)
- self.scheduler.start() # Start the scheduler
def start(self):
+ # Configure and start scheduler
+ self.scheduler = BackgroundScheduler(
+ timezone=datetime.now().astimezone().tzinfo, # Use system timezone
+ )
+ logging.getLogger('apscheduler').setLevel(logging.WARNING)
+ try:
+ self.scheduler.start()
+ except Exception as e:
+ logger.error(f"Failed to start scheduler: {e}")
+ returnLikely invalid or redundant comment.
441-442:
Add thread safety and extract common scheduling logic
- The scheduled releases cleanup needs thread safety
- The scheduling logic is duplicated between
_update_item_stateand_update_ongoing
+def _schedule_release(self, item: MediaItem, release_time: datetime) -> None:
+ """Schedule a release for the given item."""
+ with self._scheduled_releases_lock:
+ self.scheduled_releases[item.id] = release_time
+
+ job_id = f"release_{item.id}"
+ try:
+ if self.scheduler.get_job(job_id):
+ self.scheduler.remove_job(job_id)
+
+ self.scheduler.add_job(
+ self._process_release,
+ 'date',
+ run_date=release_time,
+ args=[item.id, item.log_string],
+ id=job_id,
+ name=f"Release {item.log_string}"
+ )
+ except Exception as e:
+ logger.error(f"Failed to schedule release for {item.log_string}: {e}")
def _update_item_state(self, item_id: str) -> None:
# ... existing code ...
finally:
# Remove from scheduled releases after processing
+ with self._scheduled_releases_lock:
if item_id in self.scheduled_releases and self.scheduled_releases[item_id] <= current_time:
del self.scheduled_releases[item_id]Likely invalid or redundant comment.
src/program/program.py
Outdated
| item.network = trakt_item.network | ||
| except Exception as e: | ||
| logger.error(f"Failed to fetch Trakt time for {item.log_string}: {e}") | ||
|
|
||
| # Get TVMaze time for episodes only | ||
| if item.type == "episode": | ||
| try: | ||
| tvmaze_api = di[TVMazeAPI] | ||
| show_name = item.get_top_title() | ||
| if show_name and not show_name.lower().startswith("episode "): | ||
| season_number = item.parent.number if isinstance(item, Episode) and item.parent else None | ||
| episode_number = item.number if isinstance(item, Episode) else None | ||
|
|
||
| tvmaze_time = tvmaze_api.get_show_by_imdb( | ||
| item.imdb_id, | ||
| show_name=show_name, | ||
| season_number=season_number, | ||
| episode_number=episode_number | ||
| ) | ||
| except Exception as e: | ||
| logger.error(f"Failed to fetch TVMaze time for {item.log_string}: {e}") | ||
|
|
||
| # Use earliest available time | ||
| times = [t for t in [trakt_time, tvmaze_time] if t is not None] | ||
| if not times: | ||
| continue | ||
| air_time = min(times) | ||
| if not air_time: | ||
| continue | ||
|
|
||
| # Store the air time | ||
| item.aired_at = air_time | ||
| session.merge(item) | ||
|
|
||
| # Calculate release time with delay | ||
| delay_minutes = settings_manager.settings.content.trakt.release_delay_minutes | ||
| release_time = air_time + timedelta(minutes=delay_minutes) | ||
|
|
||
| # Skip if already released | ||
| if release_time <= current_time: | ||
| if item.last_state in [States.Ongoing, States.Unreleased]: | ||
| previous_state, new_state = item.store_state() | ||
| if previous_state != new_state: | ||
| self.em.add_event(Event("StateTransition", item_id=item.id)) | ||
| logger.log("RELEASE", f"🎬 Released (late): {item.log_string}") | ||
| continue | ||
|
|
||
| # Check if releasing in next 24 hours | ||
| time_until_release = release_time - current_time | ||
| if time_until_release <= timedelta(hours=24): | ||
| release_time_str = release_time.strftime("%I:%M %p").lstrip('0') | ||
| upcoming_releases.append((item.log_string, release_time_str)) | ||
|
|
||
| # Schedule the release | ||
| self.scheduled_releases[item.id] = release_time | ||
| job_id = f"release_{item.id}" | ||
|
|
||
| try: | ||
| if self.scheduler.get_job(job_id): | ||
| self.scheduler.remove_job(job_id) | ||
|
|
||
| self.scheduler.add_job( | ||
| self._process_release, | ||
| 'date', | ||
| run_date=release_time, | ||
| args=[item.id, item.log_string], | ||
| id=job_id, | ||
| name=f"Release {item.log_string}" | ||
| ) | ||
| logger.log("PROGRAM", f"📅 Scheduled: {item.log_string} at {release_time_str}") | ||
| except Exception as e: | ||
| logger.error(f"Failed to schedule release for {item.log_string}: {e}") | ||
|
|
||
| # If this episode isn't releasing soon, skip rest of the season | ||
| if item.type == "episode": | ||
| checked_shows.add(show_id) | ||
|
|
||
| except Exception as e: | ||
| logger.error(f"Failed to process {item.log_string}: {e}") | ||
| continue | ||
|
|
||
| session.commit() | ||
|
|
||
| # Log summary of scheduled releases | ||
| if upcoming_releases: | ||
| logger.log("PROGRAM", "\n📅 Scheduled releases:") | ||
| for item_name, release_time in sorted(upcoming_releases, key=lambda x: x[1]): | ||
| logger.log("PROGRAM", f" • {item_name} at {release_time}") | ||
| else: | ||
| logger.log("PROGRAM", "No upcoming releases found") | ||
|
|
||
| except Exception as e: | ||
| logger.error(f"Failed to update state for item with ID {item_id}: {e}") | ||
| session.rollback() | ||
| logger.error(f"Database error in _update_ongoing: {e}") | ||
| raise | ||
|
|
||
| except Exception as e: | ||
| logger.error(f"Error in _update_ongoing: {e}") | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Method is too complex and needs refactoring
The _update_ongoing method is handling multiple responsibilities including API calls, database operations, and scheduling. Consider breaking it down into smaller, focused methods:
def _update_ongoing(self) -> None:
try:
current_time = datetime.now().astimezone()
logger.log("PROGRAM", "Checking for upcoming releases...")
self._clear_old_scheduled_releases(current_time)
upcoming_releases = self._process_unreleased_items(current_time)
self._log_upcoming_releases(upcoming_releases)
except Exception as e:
logger.error(f"Error in _update_ongoing: {e}")
def _clear_old_scheduled_releases(self, current_time: datetime) -> None:
with self._scheduled_releases_lock:
self.scheduled_releases = {k: v for k, v in self.scheduled_releases.items()
if v > current_time}
def _process_unreleased_items(self, current_time: datetime) -> list[tuple[str, str]]:
# Extract the main processing logic here
def _log_upcoming_releases(self, upcoming_releases: list[tuple[str, str]]) -> None:
# Extract the logging logic here🧰 Tools
🪛 Ruff (0.8.2)
250-250: Loop control variable aired_at not used within loop body
Rename unused aired_at to _aired_at
(B007)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/program/program.py (1)
441-442:⚠️ Potential issueRace condition in scheduled releases access
The access to
scheduled_releasesdictionary is not thread-safe. This could lead to race conditions in a multi-threaded environment.+from threading import Lock + class Program(threading.Thread): def __init__(self): # ... existing init code ... + self._scheduled_releases_lock = Lock() + def _update_item_state(self, item_id: str) -> None: # ... existing code ... - if item_id in self.scheduled_releases and self.scheduled_releases[item_id] <= current_time: - del self.scheduled_releases[item_id] + with self._scheduled_releases_lock: + if item_id in self.scheduled_releases and self.scheduled_releases[item_id] <= current_time: + del self.scheduled_releases[item_id]
🧹 Nitpick comments (2)
src/program/program.py (2)
250-250: Fix unused variable in loopThe loop control variable
aired_atis not used within the loop body.- for item, aired_at in items: + for item, _aired_at in items:🧰 Tools
🪛 Ruff (0.8.2)
250-250: Loop control variable
aired_atnot used within loop bodyRename unused
aired_atto_aired_at(B007)
474-474: Remove unused variableThe
midnightvariable is assigned but never used.- midnight = (current_time + timedelta(days=1)).replace(hour=0, minute=0, second=0, microsecond=0)🧰 Tools
🪛 Ruff (0.8.2)
474-474: Local variable
midnightis assigned to but never usedRemove assignment to unused variable
midnight(F841)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/program/program.py(6 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
src/program/program.py
250-250: Loop control variable aired_at not used within loop body
Rename unused aired_at to _aired_at
(B007)
474-474: Local variable midnight is assigned to but never used
Remove assignment to unused variable midnight
(F841)
🔇 Additional comments (4)
src/program/program.py (4)
444-468: LGTM! Well-structured release processing
The method has proper error handling, database session management, and logging.
605-611: LGTM! Proper cleanup implementation
The method properly handles scheduler shutdown and logging.
66-76: 🛠️ Refactor suggestion
Move scheduler start to a more appropriate location
Starting the scheduler in __init__ could be problematic if object creation fails. Consider moving the scheduler start to the start() method where other initializations occur.
def __init__(self):
super().__init__(name="Riven")
self.initialized = False
self.running = False
self.services = {}
self.enable_trace = settings_manager.settings.tracemalloc
self.em = EventManager()
self.scheduled_releases = {} # Track scheduled releases
# Configure scheduler with timezone awareness
self.scheduler = BackgroundScheduler(
timezone=datetime.now().astimezone().tzinfo, # Use system timezone
)
# Disable noisy debug logs from APScheduler
logging.getLogger('apscheduler').setLevel(logging.WARNING)
-
- self.scheduler.start() # Start the schedulerLikely invalid or redundant comment.
230-373: 🛠️ Refactor suggestion
Break down the _update_ongoing method into smaller functions
The method is handling multiple responsibilities including:
- Clearing old scheduled releases
- Fetching air times from multiple APIs
- Updating database records
- Scheduling future releases
This makes it hard to maintain and test.
def _update_ongoing(self) -> None:
+ def _fetch_air_times(self, item: MediaItem) -> tuple[datetime | None, datetime | None]:
+ trakt_time = None
+ tvmaze_time = None
+ try:
+ trakt_time = self._fetch_trakt_time(item)
+ except Exception as e:
+ logger.error(f"Failed to fetch Trakt time for {item.log_string}: {e}")
+
+ if item.type == "episode":
+ try:
+ tvmaze_time = self._fetch_tvmaze_time(item)
+ except Exception as e:
+ logger.error(f"Failed to fetch TVMaze time for {item.log_string}: {e}")
+
+ return trakt_time, tvmaze_time
+
try:
current_time = datetime.now().astimezone()
logger.log("PROGRAM", "Checking for upcoming releases...")
# ... rest of the methodLikely invalid or redundant comment.
🧰 Tools
🪛 Ruff (0.8.2)
250-250: Loop control variable aired_at not used within loop body
Rename unused aired_at to _aired_at
(B007)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/program/apis/tvmaze_api.py (1)
154-156: Simplify nested if statements.- if season_number is not None and episode_number is not None: - if hasattr(episode, 'number') and hasattr(episode, 'season'): - if episode.season == season_number and episode.number == episode_number: + if (season_number is not None and episode_number is not None and + hasattr(episode, 'number') and hasattr(episode, 'season') and + episode.season == season_number and episode.number == episode_number):🧰 Tools
🪛 Ruff (0.8.2)
154-156: Use a single
ifstatement instead of nestedifstatements(SIM102)
155-156: Use a single
ifstatement instead of nestedifstatements(SIM102)
src/program/program.py (1)
438-462: Consider adding error state handling.The method should handle cases where the release process fails by potentially setting an error state or scheduling a retry.
def _process_release(self, item_id: str, log_string: str) -> None: try: with db.Session() as session: item = session.execute( select(MediaItem).where(MediaItem.id == item_id) ).unique().scalar_one_or_none() if item: if item.last_state in [States.Ongoing, States.Unreleased]: previous_state, new_state = item.store_state() if previous_state != new_state: self.em.add_event(Event("StateTransition", item_id=item_id)) release_time = datetime.now().astimezone().strftime("%I:%M %p").lstrip('0') logger.log("RELEASE", f"🎬 Released at {release_time}: {log_string}") session.merge(item) session.commit() + else: + logger.warning(f"State transition failed for {log_string}") + # Consider setting an error state + item.last_state = States.Error + session.merge(item) + session.commit()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/program/apis/tvmaze_api.py(1 hunks)src/program/program.py(6 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
src/program/program.py
239-239: Local variable checked_shows is assigned to but never used
Remove assignment to unused variable checked_shows
(F841)
250-250: Loop control variable aired_at not used within loop body
Rename unused aired_at to _aired_at
(B007)
468-468: Local variable midnight is assigned to but never used
Remove assignment to unused variable midnight
(F841)
src/program/apis/tvmaze_api.py
154-156: Use a single if statement instead of nested if statements
(SIM102)
155-156: Use a single if statement instead of nested if statements
(SIM102)
🔇 Additional comments (7)
src/program/apis/tvmaze_api.py (4)
1-27: LGTM! Well-configured API client setup.
The TVMaze API client is properly configured with:
- Appropriate rate limiting (20 requests/10s)
- 24-hour caching
- Proper response type handling
28-39: Method documentation is clear and complete.
The docstring properly documents the method's parameters and return value.
89-146: Timezone handling is robust.
The code properly handles:
- Network timezone conversion
- UTC fallback
- Local timezone conversion
40-88: 🛠️ Refactor suggestion
Consider extracting show lookup logic into separate methods.
The show lookup logic has multiple strategies that could be extracted into separate methods for better maintainability:
+ def _lookup_show_by_name(self, show_name: str) -> Optional[Any]:
+ """Try to find show using singlesearch by name."""
+ try:
+ response = self.request_handler._request(
+ HttpMethod.GET,
+ f"{self.BASE_URL}/singlesearch/shows",
+ params={'q': show_name}
+ )
+ return response.data if response.is_ok else None
+ except HTTPError as e:
+ if e.response.status_code == 404:
+ return None
+ raise
+
+ def _lookup_show_by_imdb(self, imdb_id: str) -> Optional[Any]:
+ """Try to find show using direct IMDB lookup."""
+ try:
+ response = self.request_handler._request(
+ HttpMethod.GET,
+ f"{self.BASE_URL}/lookup/shows",
+ params={'imdb': imdb_id}
+ )
+ return response.data if response.is_ok else None
+ except HTTPError as e:
+ if e.response.status_code == 404:
+ return None
+ raiseLikely invalid or redundant comment.
src/program/program.py (3)
66-76: LGTM! Well-configured scheduler setup.
The scheduler configuration includes:
- Proper timezone awareness using system timezone
- Appropriate log level configuration
- Immediate scheduler start
599-604: LGTM! Proper scheduler shutdown.
The scheduler is correctly shutdown when stopping the program.
368-437:
Add thread safety for scheduled releases access.
The scheduled_releases dictionary is accessed from multiple threads without proper synchronization. This could lead to race conditions.
+from threading import Lock
+
class Program(threading.Thread):
def __init__(self):
super().__init__(name="Riven")
self.initialized = False
self.running = False
self.services = {}
self.enable_trace = settings_manager.settings.tracemalloc
self.em = EventManager()
self.scheduled_releases = {}
+ self._scheduled_releases_lock = Lock()
def _update_item_state(self, item_id: str) -> None:
# ... existing code ...
- if item_id in self.scheduled_releases and self.scheduled_releases[item_id] <= current_time:
- del self.scheduled_releases[item_id]
+ with self._scheduled_releases_lock:
+ if item_id in self.scheduled_releases and self.scheduled_releases[item_id] <= current_time:
+ del self.scheduled_releases[item_id]Likely invalid or redundant comment.
| item.network = trakt_item.network | ||
| except Exception as e: | ||
| logger.error(f"Failed to fetch Trakt time for {item.log_string}: {e}") | ||
|
|
||
| # Get TVMaze time for episodes only | ||
| if item.type == "episode": | ||
| try: | ||
| tvmaze_api = di[TVMazeAPI] | ||
| show_name = item.get_top_title() | ||
| if show_name and not show_name.lower().startswith("episode "): | ||
| season_number = item.parent.number if isinstance(item, Episode) and item.parent else None | ||
| episode_number = item.number if isinstance(item, Episode) else None | ||
|
|
||
| tvmaze_time = tvmaze_api.get_show_by_imdb( | ||
| item.imdb_id, | ||
| show_name=show_name, | ||
| season_number=season_number, | ||
| episode_number=episode_number | ||
| ) | ||
| except Exception as e: | ||
| logger.error(f"Failed to fetch TVMaze time for {item.log_string}: {e}") | ||
|
|
||
| # Use earliest available time | ||
| times = [t for t in [trakt_time, tvmaze_time] if t is not None] | ||
| if not times: | ||
| continue | ||
| air_time = min(times) | ||
| if not air_time: | ||
| continue | ||
|
|
||
| # Store the air time | ||
| item.aired_at = air_time | ||
| session.merge(item) | ||
|
|
||
| # Calculate release time with delay | ||
| delay_minutes = settings_manager.settings.content.trakt.release_delay_minutes | ||
| release_time = air_time + timedelta(minutes=delay_minutes) | ||
|
|
||
| # Skip if already released | ||
| if release_time <= current_time: | ||
| if item.last_state in [States.Ongoing, States.Unreleased]: | ||
| previous_state, new_state = item.store_state() | ||
| if previous_state != new_state: | ||
| self.em.add_event(Event("StateTransition", item_id=item.id)) | ||
| logger.log("RELEASE", f"🎬 Released (late): {item.log_string}") | ||
| continue | ||
|
|
||
| # Check if releasing in next 24 hours | ||
| time_until_release = release_time - current_time | ||
| if time_until_release <= timedelta(hours=24): | ||
| release_time_str = release_time.strftime("%I:%M %p").lstrip('0') | ||
| upcoming_releases.append((item.log_string, release_time_str)) | ||
|
|
||
| # Schedule the release | ||
| self.scheduled_releases[item.id] = release_time | ||
| job_id = f"release_{item.id}" | ||
|
|
||
| try: | ||
| if self.scheduler.get_job(job_id): | ||
| self.scheduler.remove_job(job_id) | ||
|
|
||
| self.scheduler.add_job( | ||
| self._process_release, | ||
| 'date', | ||
| run_date=release_time, | ||
| args=[item.id, item.log_string], | ||
| id=job_id, | ||
| name=f"Release {item.log_string}" | ||
| ) | ||
| logger.log("PROGRAM", f"📅 Scheduled: {item.log_string} at {release_time_str}") | ||
| except Exception as e: | ||
| logger.error(f"Failed to schedule release for {item.log_string}: {e}") | ||
|
|
||
| # If this episode isn't releasing soon, skip rest of the season | ||
| # if item.type == "episode": | ||
| # checked_shows.add(show_id) | ||
|
|
||
| except Exception as e: | ||
| logger.error(f"Failed to process {item.log_string}: {e}") | ||
| continue | ||
|
|
||
| session.commit() | ||
|
|
||
| # Log summary of scheduled releases | ||
| if upcoming_releases: | ||
| logger.log("PROGRAM", "\n📅 Scheduled releases:") | ||
| for item_name, release_time in sorted(upcoming_releases, key=lambda x: x[1]): | ||
| logger.log("PROGRAM", f" • {item_name} at {release_time}") | ||
| else: | ||
| logger.log("PROGRAM", "No upcoming releases found") | ||
|
|
||
| except Exception as e: | ||
| logger.error(f"Failed to update state for item with ID {item_id}: {e}") | ||
| session.rollback() | ||
| logger.error(f"Database error in _update_ongoing: {e}") | ||
| raise | ||
|
|
||
| except Exception as e: | ||
| logger.error(f"Error in _update_ongoing: {e}") | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Method is too complex and needs refactoring.
Consider breaking down this method into smaller, focused methods:
def _update_ongoing(self) -> None:
"""Update state for ongoing and unreleased items."""
try:
current_time = datetime.now().astimezone()
logger.log("PROGRAM", "Checking for upcoming releases...")
self._clear_old_scheduled_releases(current_time)
upcoming_releases = self._process_unreleased_items(current_time)
self._log_upcoming_releases(upcoming_releases)
except Exception as e:
logger.error(f"Error in _update_ongoing: {e}")
def _clear_old_scheduled_releases(self, current_time: datetime) -> None:
self.scheduled_releases = {k: v for k, v in self.scheduled_releases.items()
if v > current_time}
def _process_unreleased_items(self, current_time: datetime) -> list[tuple[str, str]]:
upcoming_releases = []
with db.Session() as session:
items = self._fetch_unreleased_items(session)
for item in items:
release_info = self._process_single_item(item, current_time, session)
if release_info:
upcoming_releases.append(release_info)
return upcoming_releasesAlso, remove unused variables:
- checked_shows = set() # Unused
- for item, aired_at in items: # aired_at unused
+ for item, _ in items:🧰 Tools
🪛 Ruff (0.8.2)
239-239: Local variable checked_shows is assigned to but never used
Remove assignment to unused variable checked_shows
(F841)
250-250: Loop control variable aired_at not used within loop body
Rename unused aired_at to _aired_at
(B007)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/program/apis/tvmaze_api.py (4)
1-27: LGTM! Consider adding API version documentation.The imports, rate limiting, and caching setup are well-structured. The rate limit of 20 requests per 10 seconds aligns with TVMaze's API limits.
Consider adding a comment or docstring mentioning which TVMaze API version this client is compatible with.
28-44: Add input validation for IMDB ID format.While the code handles the 'tt' prefix, it should validate the IMDB ID format to ensure it contains only alphanumeric characters and matches the expected pattern (e.g., 'tt' followed by 7-8 digits).
def get_show_by_imdb(self, imdb_id: str, show_name: Optional[str] = None, season_number: Optional[int] = None, episode_number: Optional[int] = None) -> Optional[datetime]: + if not imdb_id or not isinstance(imdb_id, str): + logger.error("Invalid IMDB ID: must be a non-empty string") + return None + # Add 'tt' prefix if not present if not imdb_id.startswith('tt'): imdb_id = f'tt{imdb_id}' + + # Validate IMDB ID format + if not imdb_id[2:].isalnum() or not len(imdb_id) in [9, 10]: + logger.error(f"Invalid IMDB ID format: {imdb_id}") + return None
45-88: Refactor duplicated error handling logic.The error handling pattern for HTTP 404 is repeated three times. Consider extracting it into a helper method.
+ def _make_api_request(self, endpoint: str, params: dict) -> Optional[Any]: + """Make an API request with consistent error handling.""" + try: + response = self.request_handler._request(HttpMethod.GET, f"{self.BASE_URL}/{endpoint}", params=params) + return response.data if response.is_ok else None + except HTTPError as e: + if e.response.status_code == 404: + return None + raise def get_show_by_imdb(self, imdb_id: str, show_name: Optional[str] = None, season_number: Optional[int] = None, episode_number: Optional[int] = None) -> Optional[datetime]: # ... existing code ... # Try singlesearch by show name first if show_name: logger.debug(f"Trying singlesearch by name: {show_name}") - try: - response = self.request_handler._request(HttpMethod.GET, f"{self.BASE_URL}/singlesearch/shows", params={'q': show_name}) - show = response.data if response.is_ok else None - except HTTPError as e: - if e.response.status_code == 404: - show = None - else: - raise + show = self._make_api_request("singlesearch/shows", {'q': show_name})
195-198: Consider catching specific exception types.The broad
Exceptioncatch could mask unexpected errors. Consider catching specific exceptions that you expect to handle.- except Exception as e: + except (ValueError, KeyError, AttributeError, TypeError) as e: logger.error(f"Error fetching TVMaze data for {imdb_id}: {e}") return None + except Exception as e: + logger.error(f"Unexpected error fetching TVMaze data for {imdb_id}: {e}") + raise
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/program/apis/tvmaze_api.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
src/program/apis/tvmaze_api.py
154-156: Use a single if statement instead of nested if statements
(SIM102)
🔇 Additional comments (2)
src/program/apis/tvmaze_api.py (2)
154-162: Simplify nested conditions for better readability.The nested if statements can be combined for clearer logic flow.
- if season_number is not None and episode_number is not None: - if (hasattr(episode, 'number') and hasattr(episode, 'season') and - episode.season == season_number and episode.number == episode_number): + if (season_number is not None and episode_number is not None and + hasattr(episode, 'number') and hasattr(episode, 'season') and + episode.season == season_number and episode.number == episode_number):🧰 Tools
🪛 Ruff (0.8.2)
154-156: Use a single
ifstatement instead of nestedifstatements(SIM102)
165-175: Simplify next episode tracking logic.The next episode tracking can be simplified by combining the conditions.
- if air_time > current_time: - if not next_episode or air_time < next_episode: - next_episode = air_time + if air_time > current_time and (not next_episode or air_time < next_episode): + next_episode = air_time
| for episode in sorted(episodes, key=lambda x: (getattr(x, 'season', 0), getattr(x, 'number', 0))): | ||
| try: | ||
| if not episode.airstamp: | ||
| continue | ||
|
|
||
| # First try to get air time using network timezone | ||
| air_time = None | ||
| if (hasattr(show, 'network') and show.network and | ||
| hasattr(show.network, 'country') and show.network.country and | ||
| hasattr(show.network.country, 'timezone') and show.network.country.timezone and | ||
| episode.airdate and episode.airtime): | ||
|
|
||
| # Combine airdate and airtime in network timezone | ||
| network_tz = ZoneInfo(show.network.country.timezone) | ||
| air_datetime = f"{episode.airdate}T{episode.airtime}" | ||
| try: | ||
| # Parse the time in network timezone | ||
| air_time = datetime.fromisoformat(air_datetime).replace(tzinfo=network_tz) | ||
| # Only log network time for the target episode | ||
| if (season_number is not None and episode_number is not None and | ||
| hasattr(episode, 'number') and hasattr(episode, 'season') and | ||
| episode.season == season_number and episode.number == episode_number): | ||
| logger.debug(f"Network airs show at {air_time} ({show.network.country.timezone})") | ||
| except Exception as e: | ||
| logger.error(f"Failed to parse network air time: {e}") | ||
| air_time = None | ||
|
|
||
| # Fallback to airstamp if needed | ||
| if not air_time and episode.airstamp: | ||
| try: | ||
| air_time = datetime.fromisoformat(episode.airstamp.replace('Z', '+00:00')) | ||
| if (season_number is not None and episode_number is not None and | ||
| hasattr(episode, 'number') and hasattr(episode, 'season') and | ||
| episode.season == season_number and episode.number == episode_number): | ||
| logger.debug(f"Using UTC airstamp: {air_time}") | ||
| except Exception as e: | ||
| logger.error(f"Failed to parse airstamp: {e}") | ||
| continue | ||
|
|
||
| if not air_time: | ||
| continue | ||
|
|
||
| # Convert to local time | ||
| air_time = air_time.astimezone(current_time.tzinfo) | ||
|
|
||
| # Check if this is the specific episode we want | ||
| if season_number is not None and episode_number is not None: | ||
| if (hasattr(episode, 'number') and hasattr(episode, 'season') and | ||
| episode.season == season_number and episode.number == episode_number): | ||
| # Found our target episode | ||
| if hasattr(episode, 'name'): | ||
| logger.debug(f"Found S{season_number}E{episode_number} '{episode.name}' airing at {air_time}") | ||
| else: | ||
| logger.debug(f"Found S{season_number}E{episode_number} airing at {air_time}") | ||
| target_episode_time = air_time | ||
|
|
||
| # Add all unreleased episodes to our list | ||
| if air_time > current_time: | ||
| ep_info = { | ||
| 'air_time': air_time, | ||
| 'season': getattr(episode, 'season', 0), | ||
| 'episode': getattr(episode, 'number', 0), | ||
| 'name': getattr(episode, 'name', '') | ||
| } | ||
| unreleased_episodes.append(ep_info) | ||
| # Track next episode separately | ||
| if not next_episode or air_time < next_episode: | ||
| next_episode = air_time | ||
|
|
||
| except Exception as e: | ||
| logger.error(f"Failed to process episode {getattr(episode, 'number', '?')}: {e}") | ||
| continue | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract episode processing logic into separate methods.
The episode processing loop is handling multiple responsibilities: parsing air times, finding target episodes, and tracking unreleased episodes. Consider breaking this into smaller, focused methods.
+ def _parse_network_air_time(self, show, episode) -> Optional[datetime]:
+ """Parse air time using network timezone."""
+ if not (hasattr(show, 'network') and show.network and
+ hasattr(show.network, 'country') and show.network.country and
+ hasattr(show.network.country, 'timezone') and show.network.country.timezone and
+ episode.airdate and episode.airtime):
+ return None
+
+ try:
+ network_tz = ZoneInfo(show.network.country.timezone)
+ air_datetime = f"{episode.airdate}T{episode.airtime}"
+ return datetime.fromisoformat(air_datetime).replace(tzinfo=network_tz)
+ except Exception as e:
+ logger.error(f"Failed to parse network air time: {e}")
+ return None
+ def _parse_airstamp(self, episode) -> Optional[datetime]:
+ """Parse air time from airstamp."""
+ if not episode.airstamp:
+ return None
+ try:
+ return datetime.fromisoformat(episode.airstamp.replace('Z', '+00:00'))
+ except Exception as e:
+ logger.error(f"Failed to parse airstamp: {e}")
+ return NoneCommittable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.8.2)
154-156: Use a single if statement instead of nested if statements
(SIM102)
Pull Request Check List
Resolves: #issue-number-here
Description:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Deprecations