store response headers at each requests #308
Conversation
resolve issue d60#252
Reviewer's Guide by SourceryThis PR addresses issue #252 by introducing a new mechanism to store response headers, particularly for tracking rate limit values. The implementation adds a new attribute to the client to hold the header information and updates it when a request is made. Updated Class Diagram for Client with Response HeadersclassDiagram
class Client {
- _user_id
- _user_agent
- _act_as
- _response_headers
+ __init__(...)
+ request(...)
}
note for Client "New attribute _response_headers is added to track response headers for rate limiting."
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughA new private attribute Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant S as HTTP Server
C->>S: Execute HTTP request
S-->>C: Return response (with headers)
C->>C: Set _response_headers attribute with response headers
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 (
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @bdelpey - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider using a more descriptive name than
_response_headers, such aslast_response_headers.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| self._user_id = None | ||
| self._user_agent = user_agent or 'Mozilla/5.0 (Macintosh; Intel Mac OS X 14_6_1) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.5 Safari/605.1.15' | ||
| self._act_as = None | ||
| self._response_headers = None |
There was a problem hiding this comment.
suggestion (bug_risk): Initialization of _response_headers could lead to shared mutable state issues.
Since _response_headers is an instance variable, if the client is used concurrently, multiple requests may overwrite its value. Evaluate whether this state should be tied to individual requests or managed in a thread-safe manner.
Suggested implementation:
self._act_as = None response_headers = response.headersMake sure any subsequent code in this function or other functions that previously relied on self._response_headers now uses the local variable (response_headers) or otherwise has been refactored to access the response headers as needed.
| response = await self.http.request(method, url, headers=headers, **kwargs) | ||
| self._remove_duplicate_ct0_cookie() | ||
|
|
||
| self._response_headers = response.headers |
There was a problem hiding this comment.
suggestion (bug_risk): Assignment of response headers to a shared state may be problematic in concurrent scenarios.
If the client is expected to handle concurrent requests, consider returning response headers with each request's result rather than storing them in a mutable instance variable. This can prevent potential race conditions.
Suggested implementation:
# Removed setting shared response headers to avoid race conditions. return response_data, response.headersEnsure that any callers of this function are updated to handle a tuple (data, headers) rather than expecting just the response data.
![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 (2)
twikit/client/client.py (2)
153-153: Consider adding error handling for response headers access.While storing response headers is good, consider adding error handling in case the headers are accessed before making any request. Also, consider documenting the available headers and their purpose, especially the rate limit headers that this change aims to track.
- self._response_headers = response.headers + # Store response headers with focus on rate limit information + # x-rate-limit-remaining: Number of requests remaining in the current time window + # x-rate-limit-reset: Time when the rate limit will reset (UTC epoch seconds) + self._response_headers = response.headers
119-153: Consider exposing rate limit information through a dedicated method.Since the main purpose of tracking response headers is to handle rate limits, consider adding a dedicated method to expose this information in a more user-friendly way.
+ def get_rate_limit_info(self) -> dict: + """ + Returns rate limit information from the most recent API response. + + Returns + ------- + dict + Dictionary containing: + - remaining: Number of requests remaining + - reset_time: UTC epoch seconds when the rate limit will reset + + Raises + ------ + ValueError + If no request has been made yet or rate limit headers are missing + """ + if not self._response_headers: + raise ValueError("No request has been made yet") + + remaining = self._response_headers.get('x-rate-limit-remaining') + reset_time = self._response_headers.get('x-rate-limit-reset') + + if not remaining or not reset_time: + raise ValueError("Rate limit headers not found in response") + + return { + 'remaining': int(remaining), + 'reset_time': int(reset_time) + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
twikit/client/client.py(2 hunks)
🔇 Additional comments (1)
twikit/client/client.py (1)
119-119: LGTM! Good practice initializing the headers attribute.The new private attribute
_response_headersis properly initialized to None in the constructor.
|
@bdelpey Hi, Can you give exsampel how to use this? I install it but how to get rate limit values? |
You can get all the last response headers like that : For example to store all the tweets of a particular user : |
Thank you alot - I will try it:) |
2 participants
resolve issue #252
The goal is to be able to track 'x-rate-limit-remaining' and 'x-rate-limit-reset' when retrieving a lot of tweets to avoid getting TooManyRequests exception.
Summary by Sourcery
Bug Fixes:
Summary by CodeRabbit