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

Rate limit support, multiple authentication, search using v1.1 api and more. #282

Closed
wants to merge 4 commits into from

Conversation

nirg
Copy link
Contributor

@nirg nirg commented Apr 17, 2013

Added multiple features (sorry for not separating commits):

  • API now support switching between multiple authentication handler
  • Allow blocking when reaching rate limit.
  • Search is now using Twitter API v1.1
  • 'Pagination' of timelines is now done properly with max_id parameter, applies to all endpoints supporting 'max_id'. Other endpoint that have 'page' in their argument list still use the old pagination iterator.
  • Fixed an issue with PageIterator that caused the iterator to make an extra call to the Twitter API beyond the provided page limit.
  • Add unit tests for all of the above.

- API now support switching between multiple authentication handler
- Allow blocking when reaching rate limit.
- Search is now using Twitter API v1.1
- 'Pagination' of timelines is now done properly with max_id parameter, applies to all endpoints supporting 'max_id'. Other endpoint that have 'page' in their argument list still use the old pagination iterator.
- Fixed an issue with PageIterator that caused the iterator to make an extra call to the Twitter API beyond the provided page limit.
- Add unit tests for all of the above.
Nir Grinberg added 2 commits April 27, 2013 19:06
- iterating over search result using cursor with proper pagination.
- calling user_timeline endpoint with the results parsed from JSON to python dictionary, not to other python class object.
@joshthecoder
Copy link
Member

Not sure I will accept the multiple authentications. Seems like a hack around the API rate limits.
Not saying those are your intentions, but someone will use it for those purposes.

When it comes to rate limits, I usually leave that to the application to deal with those problems.
It's not something I want to put into the scope of Tweepy since it really depends on the use case
on how those should be handled. Blocking could be totally unacceptable in a high performance server.

@nirg
Copy link
Contributor Author

nirg commented May 14, 2013

@joshthecoder I under your position regarding the multiple authentication.

Regarding blocking on reaching rate limit - it's not the default in my implementation but rather an option for the use case where you actually do want to monitor the rate limit and wait for its replenishment. Since the http response object is not exposed to the user of tweepy, she cannot monitor the remaining calls / block without first getting an error from twitter. The code in this pull request handles this scenario.

Two other issues that were addressed in this pull request are proper pagination of timelines and fix of an issue with PageIterator that caused extra call to Twitter API. Also I included examples of changing payload type and use of pagination. Please consider merging those.

@joshthecoder
Copy link
Member

Fair points. I'll have to spend more time combing though the changes and see what can go in. Thanks.

@domino14
Copy link

Can the search part be merged in as a separate pull request?

@inactivist
Copy link
Contributor

@nirg , do you have any objection to splitting your pull requests according to feature changes (better yet, one pull request/branch per issue?) I think that would streamline the review and acceptance process.

@inactivist
Copy link
Contributor

@joshthecoder wrote:

When it comes to rate limits, I usually leave that to the application to deal with those problems.
It's not something I want to put into the scope of Tweepy since it really depends on the use case
on how those should be handled. Blocking could be totally unacceptable in a high performance server.

I agree that rate limit policy should be delegated to the client application. I had the same concerns when I first read the pull request.

The ideal situation (for me anyway) is that Tweepy would provide the rate limiting info to the client and allow the client to deal with it -- but Tweepy itself should not implement the policy logic by default if at all possible.

@joshthecoder
Copy link
Member

I agree with the others having multiple features / fixes in a single request isn't ideal.
For now I am closing this request since a lot of these have either been addressed in
other requests or something there isn't much interest in (ex: multiple auth handlers).

Thanks for sharing and I would be happy to at least see the max_id/since_id based
Cursors make it into another pull request.

payload_type = 'search_result', payload_list = True,
allowed_param = ['q', 'lang', 'locale', 'rpp', 'page', 'since_id', 'geocode', 'show_user', 'max_id', 'since', 'until', 'result_type']
allowed_param = ['q', 'geocode', 'lang', 'locale', 'result_type', 'count', 'until', 'since_id', 'max_id', 'include_entities', 'rpp', 'page', 'show_user', 'since']
Copy link
Contributor

Choose a reason for hiding this comment

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

You should remove a couple of allowed_params: "page" causes tweepy to reply with an error, "rpp" and "show_user" are ignored but don't generate errors (at the moment, anyway..). "cursor" doesn't really make sense here, and is also ignored (but doesn't generate an error)

@ghost
Copy link

ghost commented Feb 6, 2017

Hi @nirg do you have an idea what I am doing wrong? I am getting an error when trying to switch to the next auth: http://stackoverflow.com/questions/42059191/tweepy-multiple-auth-handler

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants