Add AccountLimitExceeded error #63
Conversation
WalkthroughThe pull request introduces a new exception class Changes
Possibly related PRs
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: 0
🧹 Nitpick comments (1)
trakt/errors.py (1)
147-155: Good implementation, consider adding property documentation and error handling.The implementation follows module patterns well and makes a good choice inheriting from
RateLimitException. Consider these improvements:
- Add docstring for the
account_limitproperty explaining the header's purpose and possible values- Handle missing headers consistently with other properties
class AccountLimitExceeded(RateLimitException): """TraktException type to be raised when a 420 return code is received""" http_code = 420 message = 'Account Limit Exceeded - list count, item count, etc' @property def account_limit(self): + """Get the account limit details from response headers. + + Returns: + str|None: The value of x-account-limit header or None if not present + """ - return self.response.headers.get("x-account-limit") + return self.response.headers.get("x-account-limit", None)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
trakt/errors.py(2 hunks)
🔇 Additional comments (2)
trakt/errors.py (2)
18-18: LGTM! Export of new exception class follows module patterns.The addition to
__all__maintains alphabetical ordering and correctly exposes the new exception class.
147-155: Verify HTTP 420 status code in Trakt API documentation.The implementation assumes HTTP 420 for account limits, but this isn't listed in the status codes comment at the top of the file (which references https://trakt.docs.apiary.io/#introduction/status-codes).
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
trakt/errors.py (3)
18-18: Maintain consistent organization in__all__list.The new exception should be grouped with other HTTP status code exceptions to maintain the existing organization pattern.
Move the
AccountLimitExceededentry after the comment "# Exceptions by HTTP status code" to maintain consistency:- 'AccountLimitExceeded', # Exceptions by HTTP status code # https://trakt.docs.apiary.io/#introduction/status-codes + 'AccountLimitExceeded', 'BadRequestException',
152-154: Consider handling missing headers.The
account_limitproperty should handle cases where the header is missing, similar to how the parent class'sdetailsproperty handles invalid JSON.Consider adding a default value:
@property def account_limit(self): - return self.response.headers.get("x-account-limit") + return self.response.headers.get("x-account-limit", None)
155-156: Remove extra blank line.There's an unnecessary extra blank line before the next class definition.
def account_limit(self): return self.response.headers.get("x-account-limit") - class TraktInternalException(TraktException):
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
trakt/errors.py(2 hunks)
🔇 Additional comments (1)
trakt/errors.py (1)
147-154: LGTM! Well-structured exception class.The implementation is clean and follows the module's patterns. Good choice inheriting from
RateLimitExceptionto reuse existing rate limit functionality.
|
Tested successfully ✅ |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
trakt/errors.py (1)
18-18: Reorder exceptions list by HTTP status codes.The
AccountLimitExceededexception should be placed betweenLockedUserAccountException(423) andRateLimitException(429) to maintain the existing ordering by HTTP status codes.# Exceptions by HTTP status code # https://trakt.docs.apiary.io/#introduction/status-codes - 'AccountLimitExceeded', 'BadRequestException', 'OAuthException', 'ForbiddenException', 'NotFoundException', 'MethodNotAllowedException', 'ConflictException', 'ProcessException', 'LockedUserAccountException', + 'AccountLimitExceeded', 'RateLimitException', 'TraktInternalException', 'TraktBadGateway', 'TraktUnavailable',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
trakt/errors.py(2 hunks)
🔇 Additional comments (1)
trakt/errors.py (1)
147-160: LGTM! Well-structured exception class.The implementation is clean, well-documented, and follows the module's patterns. The class correctly inherits from
RateLimitExceptionand adds theaccount_limitproperty for accessing the response header.
Refs