-
Notifications
You must be signed in to change notification settings - Fork 5
Fix: Set max retries to 1 #61
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
Conversation
WalkthroughThe pull request modifies the 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 (
|
Getting banned by trakt api if doing more than once
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
trakt/api.py (1)
176-176: Consider implementing proper rate limitingTo properly address API bans, consider implementing:
- Rate limiting based on Trakt API guidelines
- Respect for rate limit headers in responses
- Exponential backoff for failed requests
Would you like me to:
- Generate a proper rate limiting implementation?
- Open a GitHub issue to track this enhancement?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
trakt/api.py(1 hunks)
🔇 Additional comments (1)
trakt/api.py (1)
176-176:⚠️ Potential issueMisaligned solution: Token refresh attempts vs API request retries
The change reduces OAuth token refresh attempts, but this won't address the API ban issue. Token refresh:
- Uses a different endpoint (/oauth/token) that's excluded from regular API calls
- Only occurs when the access token is near expiry
- Is part of the OAuth2 flow, not regular API requests
Reducing token refresh attempts could lead to more frequent authentication failures requiring manual intervention, potentially causing more API calls when users need to re-authenticate.
Let's verify the actual cause of API bans. Run this script to:
- Find the request retry mechanism
- Check for rate limiting headers
- Look for API call patterns
Recommendation: Instead of modifying token refresh attempts, we should:
- Verify if the API bans are caused by request retries
- Investigate implementing proper rate limiting
- Consider adding exponential backoff for failed requests
Getting banned by trakt api if doing more than once.