[BUG] Fix recently-added webhook parsing#248
Conversation
… data parsing - Update documentation for webhook payload setup
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a Pydantic pre-validator and a lightweight base model (_Base and IntAsString), switches several models to inherit from _Base, expands Tautulli webhook data schema with new fields/properties, adds typing-extensions to requirements, and adjusts a JSON example placeholder format in docs. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
modules/webhooks/tautulli_recently_added.py (1)
42-46: Duration parsing may fail on "HH:MM:SS" format.The unpacking on line 45 assumes exactly two parts after splitting on
:. If Tautulli ever sends duration in "HH:MM:SS" format, this will raiseValueError: too many values to unpack.🔎 Proposed defensive fix
if ':' not in self.duration_: return int(self.duration_) - hours, minutes = self.duration_.split(':') - return int(hours) * 60 + int(minutes) + parts = self.duration_.split(':') + if len(parts) == 2: + hours, minutes = parts + return int(hours) * 60 + int(minutes) + elif len(parts) == 3: + hours, minutes, _ = parts # Ignore seconds + return int(hours) * 60 + int(minutes) + return None
🧹 Nitpick comments (3)
requirements.txt (1)
14-14: Consider adding a version constraint for typing-extensions.The
typing-extensionsdependency is added without a version constraint. While the features used (Annotated) are available in older versions, specifying a minimum version ensures compatibility and prevents potential issues.🔎 Suggested version constraint
-typing-extensions +typing-extensions>=4.0.0modules/models/_base.py (1)
23-23: Consider extracting error messages to exception class.Static analysis suggests moving the error message strings into custom exception classes for better maintainability and reusability.
🔎 Optional refactor
+class InvalidIntStringError(ValueError): + """Raised when a value cannot be coerced to a valid int string.""" + pass + + def int_to_string(value: Any) -> Union[str, None]: """ Handle coercing ints to strings for Pydantic models. :param value: The value to be converted. :return: The string representation of the int, or None if the value is None. """ if value is None: return None if isinstance(value, int): return str(value) if isinstance(value, str): try: int(value) except ValueError as e: - raise ValueError("String value cannot be converted to int") from e + raise InvalidIntStringError("String value cannot be converted to int") from e return value - raise ValueError("Input value is not an int or string") + raise InvalidIntStringError("Input value is not an int or string")Also applies to: 26-26
modules/webhooks/tautulli_recently_added.py (1)
84-89: Consider narrowing the exception handling.Catching all exceptions here could mask unexpected errors (e.g.,
MemoryError,RecursionError). Consider catching more specific exceptions likeBadRequestor JSON-related exceptions from Flask/Werkzeug.🔎 Suggested improvement
+from werkzeug.exceptions import BadRequest + try: body = request.get_json() - except Exception: + except (BadRequest, TypeError, ValueError): # JSON data is stored in the form field 'payload_json' if files are present # ref: https://github.com/Tautulli/Tautulli/blob/d019efcf911b4806618761c2da48bab7d04031ec/plexpy/notifiers.py#L1225 body = json.loads(request.form.get('payload_json', '{}'))
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
documentation/DOCUMENTATION.mdmodules/models/_base.pymodules/models/recently_added_item.pymodules/models/webhook.pymodules/webhooks/tautulli_recently_added.pyrequirements.txt
🧰 Additional context used
🧬 Code graph analysis (2)
modules/models/webhook.py (2)
modules/models/_base.py (1)
_Base(32-33)modules/database/models/webhooks.py (1)
Webhook(7-17)
modules/models/recently_added_item.py (2)
modules/models/_base.py (1)
_Base(32-33)modules/database/models/recently_added_item.py (1)
RecentlyAddedItem(6-20)
🪛 Ruff (0.14.10)
modules/models/_base.py
23-23: Avoid specifying long messages outside the exception class
(TRY003)
26-26: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (9)
documentation/DOCUMENTATION.md (1)
373-388: LGTM! Documentation correctly updated.The webhook JSON data format has been updated to use single-brace placeholders (e.g.,
{title}) instead of double-brace format (e.g.,{{title}}), which aligns with standard Tautulli webhook variable syntax.modules/models/webhook.py (1)
4-7: LGTM! Clean migration to shared base class.The model now inherits from
_Baseinstead of directly fromBaseModel, enabling shared validation and typing utilities across the codebase while maintaining the same functionality.modules/models/recently_added_item.py (1)
2-5: LGTM! Clean migration to shared base class.The model now inherits from
_Baseinstead of directly fromBaseModel, consistent with the broader refactoring effort to use shared validation utilities.modules/models/_base.py (2)
7-26: Function logic is correct for int/string coercion.The
int_to_stringfunction properly handles the three expected cases:
- Passes
Nonethrough unchanged- Converts
intvalues tostr- Validates that
strvalues represent valid integers before accepting themThis enables the
IntAsStringtype to accept both integer and string inputs while ensuring type safety.
32-33: LGTM! Minimal base class provides extension point.The empty
_Baseclass serves as a shared foundation for models across the codebase, allowing future common validation logic or configuration to be added in one place.modules/webhooks/tautulli_recently_added.py (4)
4-7: LGTM!Import changes align with the migration to the custom
_Basemodel andIntAsStringtype alias for handling int-to-string coercion.
48-66: LGTM!The property implementations for
directors,actors,genres,critic_rating, andaudience_ratingcorrectly handleNonecases and use appropriate type conversions.
100-101: LGTM!The
_determine_triggermethod correctly returns theRECENTLY_ADDEDtrigger constant.
17-17: > Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
modules/models/_base.py
🧰 Additional context used
🪛 Ruff (0.14.10)
modules/models/_base.py
23-23: Avoid specifying long messages outside the exception class
(TRY003)
26-26: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (2)
modules/models/_base.py (2)
1-4: LGTM!Imports are appropriate and necessary for the Pydantic validator implementation.
32-33: LGTM!The minimal base class provides a clean foundation for extending Pydantic models across the codebase.
1 participant
Summary by CodeRabbit
Documentation
New Features
Refactor
Dependencies
✏️ Tip: You can customize this high-level summary in your review settings.