-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
add refresh_token function #1804
base: master
Are you sure you want to change the base?
Conversation
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.
OAuth2Session.refresh_token()
has a different signature from what you're using. I'm pretty sure this is going to break OAuth2Session
's ability to do automatic token refresh.
tweepy/auth.py
Outdated
@@ -226,3 +226,12 @@ def fetch_token(self, authorization_response): | |||
include_client_id=True, | |||
code_verifier=self.code_verifier | |||
) | |||
|
|||
def refresh_token(self, refresh_token): |
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.
You are changing the signature of a method that is also used internally. I can't imagine this going over well.
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.
@ewtoombs Thank you for noticing.
I fixed the problem.
I have also been working on token refresh. My solution (https://github.com/tweepy/tweepy/pull/2039/files) uses requests-oauthlib's built-in support for token autorefresh. It works well, but is probably incompatible with your work, and I'm not sure how to resolve this. Could you have a look? |
@ewtoombs Your auto-refresh solution is great! But manually refresh is needed too(ex: save token file when refresh). I think my code has resolved the conflict and is compatible with your solution, but am I missing something? |
My solution actually does support saving the token to a file on refresh. See the new
I didn't provide a way to turn off token autorefresh. If manual refresh is really necessary, I'd have to think of a way to do this. At that point, your change should be compatible with mine. But I am really stuggling to think of a case where manual refresh would be necessary. |
manually refresh is needed. For example, one token is used by two or more servers. |
Ooh. OK, this is getting more complicated. I don't understand this example, but I'm just going to take your word for it. I'll implement a boolean argument to the constructor that allows autorefresh to be disabled. There may be other concerns about compatibility between our modifications, though. I passed the twitter-specific modifications in a different way from how you did it. It is possible that your manual refresh function, To be continued. |
Sorry, but I've run out of time for this, @yamahubuki . |
I added refresh_token function.
It need to make an application that Operate for a long time.