Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: adding tvmaze for accurate release time detection #923

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open
9 changes: 8 additions & 1 deletion src/alembic/env.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import logging

from loguru import logger
from sqlalchemy import engine_from_config, pool, text
from sqlalchemy import engine_from_config, pool, text, event
from sqlalchemy.exc import OperationalError, ProgrammingError

from alembic import context
Expand Down Expand Up @@ -74,6 +74,13 @@ def run_migrations_online() -> None:
poolclass=pool.NullPool,
)

# Set PostgreSQL to handle case-sensitive identifiers
@event.listens_for(connectable, "connect")
def set_postgresql_case_sensitive(dbapi_connection, connection_record):
cursor = dbapi_connection.cursor()
cursor.execute("SET quote_all_identifiers = true;")
cursor.close()

Copy link
Contributor

@coderabbitai coderabbitai bot Nov 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling to the PostgreSQL event listener.

The event listener should handle potential database errors to prevent silent failures.

Consider wrapping the cursor operations in a try-finally block:

 @event.listens_for(connectable, "connect")
 def set_postgresql_case_sensitive(dbapi_connection, connection_record):
     cursor = dbapi_connection.cursor()
-    cursor.execute("SET quote_all_identifiers = true;")
-    cursor.close()
+    try:
+        cursor.execute("SET quote_all_identifiers = true;")
+    except Exception as e:
+        logger.error(f"Failed to set PostgreSQL case sensitivity: {e}")
+        raise
+    finally:
+        cursor.close()
📝 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.

Suggested change
# Set PostgreSQL to handle case-sensitive identifiers
@event.listens_for(connectable, "connect")
def set_postgresql_case_sensitive(dbapi_connection, connection_record):
cursor = dbapi_connection.cursor()
cursor.execute("SET quote_all_identifiers = true;")
cursor.close()
# Set PostgreSQL to handle case-sensitive identifiers
@event.listens_for(connectable, "connect")
def set_postgresql_case_sensitive(dbapi_connection, connection_record):
cursor = dbapi_connection.cursor()
try:
cursor.execute("SET quote_all_identifiers = true;")
except Exception as e:
logger.error(f"Failed to set PostgreSQL case sensitivity: {e}")
raise
finally:
cursor.close()

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove all the env.py changes

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dreulavelle, understood. I'll disregard the changes in env.py.


✏️ Learnings added
Learnt from: dreulavelle
PR: rivenmedia/riven#923
File: src/alembic/env.py:77-83
Timestamp: 2024-12-06T01:28:31.170Z
Learning: Avoid suggesting changes to `src/alembic/env.py` in future code reviews unless explicitly required.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

with connectable.connect() as connection:
connection = connection.execution_options(isolation_level="AUTOCOMMIT")
try:
Expand Down
6 changes: 6 additions & 0 deletions src/program/apis/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from .overseerr_api import OverseerrAPI, OverseerrAPIError
from .plex_api import PlexAPI, PlexAPIError
from .trakt_api import TraktAPI, TraktAPIError
from .tvmaze_api import TVMazeAPI, TVMazeAPIError


def bootstrap_apis():
Expand All @@ -15,6 +16,7 @@ def bootstrap_apis():
__setup_mdblist()
__setup_overseerr()
__setup_listrr()
__setup_tvmaze()
dreulavelle marked this conversation as resolved.
Show resolved Hide resolved

def __setup_trakt():
traktApi = TraktAPI(settings_manager.settings.content.trakt)
Expand Down Expand Up @@ -43,3 +45,7 @@ def __setup_listrr():
return
listrrApi = ListrrAPI(settings_manager.settings.content.listrr.api_key)
di[ListrrAPI] = listrrApi

def __setup_tvmaze():
tvmazeApi = TVMazeAPI()
di[TVMazeAPI] = tvmazeApi
39 changes: 36 additions & 3 deletions src/program/apis/trakt_api.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import re
from datetime import datetime
from datetime import datetime, timedelta, timezone
import time
from zoneinfo import ZoneInfo
from tzlocal import get_localzone
from types import SimpleNamespace
from typing import List, Optional, Union
from urllib.parse import urlencode
Expand Down Expand Up @@ -58,6 +61,18 @@ def __init__(self, settings: TraktModel):
}
session.headers.update(self.headers)
self.request_handler = TraktRequestHandler(session)

# Get timezone by comparing local time with UTC
local_time = datetime.now()
utc_time = datetime.now(timezone.utc)
offset = local_time.hour - utc_time.hour

# Create timezone with the calculated offset
try:
self.local_tz = timezone(timedelta(hours=offset))
except Exception:
self.local_tz = timezone.utc
logger.warning("Could not determine system timezone, using UTC")
dreulavelle marked this conversation as resolved.
Show resolved Hide resolved

def validate(self):
return self.request_handler.execute(HttpMethod.GET, f"{self.BASE_URL}/lists/2")
Expand Down Expand Up @@ -360,7 +375,25 @@ def _get_imdb_id_from_list(self, namespaces: List[SimpleNamespace], id_type: str
def _get_formatted_date(self, data, item_type: str) -> Optional[datetime]:
"""Get the formatted aired date from the data."""
if item_type in ["show", "season", "episode"] and (first_aired := getattr(data, "first_aired", None)):
return datetime.strptime(first_aired, "%Y-%m-%dT%H:%M:%S.%fZ")
try:
# First try with milliseconds
utc_dt = datetime.strptime(first_aired, "%Y-%m-%dT%H:%M:%S.%fZ").replace(tzinfo=timezone.utc)
return utc_dt.astimezone(self.local_tz)
except ValueError:
try:
# Try without milliseconds
utc_dt = datetime.strptime(first_aired, "%Y-%m-%dT%H:%M:%SZ").replace(tzinfo=timezone.utc)
return utc_dt.astimezone(self.local_tz)
except ValueError as e:
logger.error(f"Failed to parse Trakt air date: {first_aired} - {e}")
return None

if item_type == "movie" and (released := getattr(data, "released", None)):
return datetime.strptime(released, "%Y-%m-%d")
try:
# For movies, Trakt provides date only, set to midnight in user's timezone
local_midnight = datetime.strptime(released, "%Y-%m-%d").replace(tzinfo=self.local_tz)
return local_midnight
except ValueError as e:
logger.error(f"Failed to parse Trakt release date: {released} - {e}")
return None
return None
206 changes: 206 additions & 0 deletions src/program/apis/tvmaze_api.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,206 @@
"""TVMaze API client module"""

from datetime import datetime, timedelta, timezone
from typing import Optional

from loguru import logger
from requests import Session

from program.media.item import Episode, MediaItem
from program.utils.request import (
BaseRequestHandler,
HttpMethod,
ResponseType,
create_service_session,
get_cache_params,
get_rate_limit_params,
)

class TVMazeAPIError(Exception):
"""Base exception for TVMaze API related errors"""

class TVMazeRequestHandler(BaseRequestHandler):
def __init__(self, session: Session, response_type=ResponseType.SIMPLE_NAMESPACE, request_logging: bool = False):
super().__init__(session, response_type=response_type, custom_exception=TVMazeAPIError, request_logging=request_logging)

def execute(self, method: HttpMethod, endpoint: str, **kwargs):
return super()._request(method, endpoint, **kwargs)

class TVMazeAPI:
"""Handles TVMaze API communication"""
BASE_URL = "https://api.tvmaze.com"

def __init__(self):
rate_limit_params = get_rate_limit_params(max_calls=20, period=10)
tvmaze_cache = get_cache_params("tvmaze", 86400)
session = create_service_session(
rate_limit_params=rate_limit_params,
use_cache=True,
cache_params=tvmaze_cache
)
self.request_handler = TVMazeRequestHandler(session)

# Get timezone by comparing local time with UTC
local_time = datetime.now()
utc_time = datetime.now(timezone.utc)
offset = local_time.hour - utc_time.hour

# Create timezone with the calculated offset
try:
self.local_tz = timezone(timedelta(hours=offset))
except Exception:
self.local_tz = timezone.utc
logger.warning("Could not determine system timezone, using UTC")

dreulavelle marked this conversation as resolved.
Show resolved Hide resolved
def get_show_by_imdb_id(self, imdb_id: str) -> Optional[dict]:
"""Get show information by IMDb ID"""
if not imdb_id:
return None

url = f"{self.BASE_URL}/lookup/shows"
response = self.request_handler.execute(
HttpMethod.GET,
url,
params={"imdb": imdb_id}
)
if not response.is_ok:
return None

show_url = f"{self.BASE_URL}/shows/{response.data.id}"
show_response = self.request_handler.execute(HttpMethod.GET, show_url)
return show_response.data if show_response.is_ok else None
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Avoid Redundant API Call in get_show_by_imdb_id

The method makes two API calls: one to /lookup/shows and then another to /shows/{id}. If the initial response from /lookup/shows contains all the necessary show information, the second API call may be unnecessary. Verify if the data returned by /lookup/shows suffices, and eliminate the redundant API call to improve performance.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wolfemir you should take a look at this one, he's got a point ;)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!


def get_episode_by_number(self, show_id: int, season: int, episode: int) -> Optional[datetime]:
"""Get episode information by show ID and episode number"""
if not show_id or not season or not episode:
return None

url = f"{self.BASE_URL}/shows/{show_id}/episodebynumber"
response = self.request_handler.execute(
HttpMethod.GET,
url,
params={
"season": season,
"number": episode
}
)

if not response.is_ok or not response.data:
return None

return self._parse_air_date(response.data)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance error handling for API calls

The method should handle specific API exceptions similar to get_show_by_imdb_id.

 def get_episode_by_number(self, show_id: int, season: int, episode: int) -> Optional[datetime]:
     if not show_id or not season or not episode:
         return None
 
     url = f"{self.BASE_URL}/shows/{show_id}/episodebynumber"
-    response = self.request_handler.execute(
-        HttpMethod.GET,
-        url,
-        params={
-            "season": season,
-            "number": episode
-        }
-    )
-    
-    if not response.is_ok or not response.data:
+    try:
+        response = self.request_handler.execute(
+            HttpMethod.GET,
+            url,
+            params={
+                "season": season,
+                "number": episode
+            }
+        )
+        
+        if not response.is_ok or not response.data:
+            return None
+
+        return self._parse_air_date(response.data)
+    except Exception as e:
+        logger.debug(f"Error getting episode by number - show_id={show_id}, S{season:02d}E{episode:02d}: {e}")
         return None
-
-    return self._parse_air_date(response.data)
📝 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.

Suggested change
def get_episode_by_number(self, show_id: int, season: int, episode: int) -> Optional[datetime]:
"""Get episode information by show ID and episode number"""
if not show_id or not season or not episode:
return None
url = f"{self.BASE_URL}/shows/{show_id}/episodebynumber"
response = self.request_handler.execute(
HttpMethod.GET,
url,
params={
"season": season,
"number": episode
}
)
if not response.is_ok or not response.data:
return None
return self._parse_air_date(response.data)
def get_episode_by_number(self, show_id: int, season: int, episode: int) -> Optional[datetime]:
"""Get episode information by show ID and episode number"""
if not show_id or not season or not episode:
return None
url = f"{self.BASE_URL}/shows/{show_id}/episodebynumber"
try:
response = self.request_handler.execute(
HttpMethod.GET,
url,
params={
"season": season,
"number": episode
}
)
if not response.is_ok or not response.data:
return None
return self._parse_air_date(response.data)
except Exception as e:
logger.debug(f"Error getting episode by number - show_id={show_id}, S{season:02d}E{episode:02d}: {e}")
return None


def _parse_air_date(self, episode_data) -> Optional[datetime]:
"""Parse episode air date from TVMaze response"""
if airstamp := getattr(episode_data, "airstamp", None):
try:
# Handle both 'Z' suffix and explicit timezone
timestamp = airstamp.replace('Z', '+00:00')
if '.' in timestamp:
# Strip milliseconds but preserve timezone
parts = timestamp.split('.')
base = parts[0]
tz = parts[1][parts[1].find('+'):]
timestamp = base + tz if '+' in parts[1] else base + '+00:00'
elif not ('+' in timestamp or '-' in timestamp):
# Add UTC timezone if none specified
timestamp = timestamp + '+00:00'
# Convert to user's timezone
utc_dt = datetime.fromisoformat(timestamp)
return utc_dt.astimezone(self.local_tz)
except (ValueError, AttributeError) as e:
logger.debug(f"Failed to parse TVMaze airstamp: {airstamp} - {e}")

try:
if airdate := getattr(episode_data, "airdate", None):
if airtime := getattr(episode_data, "airtime", None):
# Combine date and time with UTC timezone first
dt_str = f"{airdate}T{airtime}+00:00"
utc_dt = datetime.fromisoformat(dt_str)
# Convert to user's timezone
return utc_dt.astimezone(self.local_tz)
# If we only have a date, set time to midnight in user's timezone
local_midnight = datetime.fromisoformat(f"{airdate}T00:00:00").replace(tzinfo=self.local_tz)
return local_midnight
except (ValueError, AttributeError) as e:
logger.error(f"Failed to parse TVMaze air date/time: {airdate}/{getattr(episode_data, 'airtime', None)} - {e}")

return None

def get_episode_release_time(self, item: MediaItem) -> Optional[datetime]:
"""Get episode release time for a media item"""
if not isinstance(item, Episode):
logger.debug(f"Skipping release time lookup - not an episode: {item.log_string if item else None}")
return None

if not item.parent or not item.parent.parent:
logger.debug(f"Skipping release time lookup - missing parent/show: {item.log_string}")
return None

if not item.parent.parent.imdb_id:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of all the if not item.parent.parent stuff, you can check the type by doing if item.type == "episode"

doing it like it is now can cause errors to popup when an item doesn't have the nested parent attributes

logger.debug(f"Skipping release time lookup - no IMDb ID: {item.log_string}")
return None

show = self.get_show_by_imdb_id(item.parent.parent.imdb_id)
if not show or not hasattr(show, "id"):
logger.debug(f"Failed to get TVMaze show for IMDb ID: {item.parent.parent.imdb_id}")
return None

# Get episode air date from regular schedule
air_date = self.get_episode_by_number(show.id, item.parent.number, item.number)
if air_date:
logger.debug(f"Found regular schedule time for {item.log_string}: {air_date}")

# Check streaming releases for next 30 days
today = datetime.now(self.local_tz).replace(hour=0, minute=0, second=0, microsecond=0)
logger.debug(f"Checking streaming schedule for {item.log_string} (Show ID: {show.id})")

for i in range(30):
check_date = today + timedelta(days=i)
url = f"{self.BASE_URL}/schedule/web"
response = self.request_handler.execute(
HttpMethod.GET,
url,
params={
"date": check_date.strftime("%Y-%m-%d"),
"country": None
}
)

if not response.is_ok:
logger.debug(f"Failed to get streaming schedule for {check_date.strftime('%Y-%m-%d')}")
continue

for release in response.data:
if not release:
continue

show_data = getattr(release, "show", None)
if not show_data:
continue

# Get externals safely
externals = getattr(show_data, "externals", {}) or {}
show_imdb = externals.get("imdb")
show_id = getattr(show_data, "id", None)

# Check both IMDb ID and TVMaze show ID
is_match = (
(show_imdb == item.parent.parent.imdb_id or show_id == show.id) and
getattr(release, "season", 0) == item.parent.number and
getattr(release, "number", 0) == item.number
)

if is_match:
streaming_date = self._parse_air_date(release)
if streaming_date:
logger.debug(f"Found streaming release for {item.log_string}: {streaming_date}")
if not air_date or streaming_date < air_date:
air_date = streaming_date
logger.debug(f"Using earlier streaming release time for {item.log_string}: {streaming_date}")

if air_date:
logger.debug(f"Final release time for {item.log_string}: {air_date}")
else:
logger.debug(f"No release time found for {item.log_string}")
return air_date
19 changes: 16 additions & 3 deletions src/program/media/item.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,18 @@ def blacklist_stream(self, stream: Stream):
@property
def is_released(self) -> bool:
"""Check if an item has been released."""
if self.aired_at and self.aired_at <= datetime.now():
return True
return False
if not self.aired_at:
return False

# Ensure both datetimes are timezone-aware for comparison
now = datetime.now().astimezone()
aired_at = self.aired_at

# Make aired_at timezone-aware if it isn't already
if aired_at.tzinfo is None:
aired_at = aired_at.replace(tzinfo=now.tzinfo)

return aired_at <= now
Comment on lines +180 to +191
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Properly handle naive aired_at datetime objects to ensure accurate timezone conversions

Directly setting the timezone on a naive datetime object using replace(tzinfo=...) assumes that the original datetime is in the local timezone, which may not be accurate. This can lead to incorrect release status determination.

Apply this diff to correctly localize aired_at assuming it is in UTC:

+    from datetime import timezone
     ...
     if aired_at.tzinfo is None:
-        aired_at = aired_at.replace(tzinfo=now.tzinfo)
+        aired_at = aired_at.replace(tzinfo=timezone.utc).astimezone(now.tzinfo)

Alternatively, if aired_at is intended to be in the local timezone, consider using a method that properly localizes naive datetime objects without assuming they are in UTC.

📝 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.

Suggested change
if not self.aired_at:
return False
# Ensure both datetimes are timezone-aware for comparison
now = datetime.now().astimezone()
aired_at = self.aired_at
# Make aired_at timezone-aware if it isn't already
if aired_at.tzinfo is None:
aired_at = aired_at.replace(tzinfo=now.tzinfo)
return aired_at <= now
if not self.aired_at:
return False
# Ensure both datetimes are timezone-aware for comparison
from datetime import timezone
now = datetime.now().astimezone()
aired_at = self.aired_at
# Make aired_at timezone-aware if it isn't already
if aired_at.tzinfo is None:
aired_at = aired_at.replace(tzinfo=timezone.utc).astimezone(now.tzinfo)
return aired_at <= now


@property
def state(self):
Expand Down Expand Up @@ -391,6 +400,9 @@ def _reset(self):
def log_string(self):
return self.title or self.id

def __repr__(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this, its not needed as item.log_string already does it for us

return f"MediaItem:{self.log_string}:{self.state.name}"

@property
def collection(self):
return self.parent.collection if self.parent else self.id
Expand Down Expand Up @@ -420,6 +432,7 @@ def __repr__(self):
def __hash__(self):
return super().__hash__()


class Show(MediaItem):
"""Show class"""
__tablename__ = "Show"
Expand Down
Loading