-
-
Notifications
You must be signed in to change notification settings - Fork 12
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
feat(Threads): Refresh Threads access tokens automatically. #629
base: main
Are you sure you want to change the base?
Conversation
bc/channel/tasks.py
Outdated
# Schedule the next token refresh | ||
delay_seconds = ( | ||
expires_in - 86400 | ||
) # Subtract one day to avoid expiration before the task runs | ||
queue.enqueue_in( | ||
timedelta(seconds=delay_seconds if delay_seconds > 0 else expires_in), | ||
refresh_threads_access_token, | ||
channel_pk=channel.pk, | ||
retry=Retry( | ||
max=settings.RQ_MAX_NUMBER_OF_RETRIES, | ||
interval=settings.RQ_RETRY_INTERVAL, | ||
), | ||
) |
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.
As discussed with @ERosendo, although enqueueing a job within the execution of another job isn't the most elegant solution, it's good enough for this use case since the first one will most definitely have finished executing way before the second begins.
bc/channel/tasks.py
Outdated
"grant_type": "th_refresh_token", | ||
"access_token": channel.access_token, | ||
} | ||
response = requests.get(refresh_access_token_url, params=params) |
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.
Please don't forget a timeout. It'll haunt you later.
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.
This looks generally OK, but I think we need something else to trigger expiration because this creates an ongoing chain of refreshes that's bound to break eventually. If just one refresh fails for whatever reason (say threads is down?), we'll never refresh again and we'll be sad.
Could you check the token's expiration every time you do a post and trigger a refresh if you're getting close to the deadline? You could get fancy and put a semaphore in redis to prevent two or more simultaneous refresh tasks, but I'm not sure it matters. Ideally we're sending one or two posts per hour, say, and so you get:
- New post
- Refresh task queued
- Refresh task completed
- Next post
You could have two or more posts at the same time, I suppose, which would lead to:
- New post
- First refresh task queued
- Next post
- Second refresh task queued
- First refresh complete
- Second refresh complete
That's fine too, right?
We did consider an approach like the one you're suggesting but the only way to get the tokens' expiration is via token retrieval, so we have no way of getting the expiration from the API when publishing posts. @ERosendo and I were discussing some alternatives to achieve something similar by storing the expiration date somewhere (so we can check that every time we do a post), either in our database, or on redis, and they both have their pros and cons:
Another alternative entirely could be to keep the chain of refreshes, and add redundancy by using a lazy refresh on post if we get an authentication error from Threads API. Of course this also has its pros and cons:
I personally like the redis option because it seems more elegant and it's something I'd like to learn about, but the database option with a new model seems like the safest bet? It's probably best for persistence and reliability. The redundancy alternative is not my favorite but it seems simpler to implement at first glance. What do you think? |
Co-authored-by: Mike Lissner <[email protected]>
Redis is plenty persistent for something like this, and you could have a rule that says, "If the key is missing, just refresh the key." If you do that, I think you'd be good to go without having to fiddle with the models. We do have the redis CLI installed and the redis Python module is available, so you wouldn't need to convert it to a Django command to just make/check a key. |
… logic - Eliminated chained enqueued tasks used for token refreshing in Threads integration. - Moved token validation and refresh logic into the Channel model's validate_access_token method. - Adjusted ThreadsAPI methods to handle token expiration and refreshing internally. - Implemented Redis-based locking mechanism to prevent concurrent token refreshes.
Resolves #627
Long-lived tokens need to be at least 24 hours old to be refreshed so upon creation of a new Threads channel, a
post_save
signal enqueues a job to refresh the token in 2 days. After that, we enqueue a new job in as many seconds as theexpires_in
value from the token refresh response.