Skip to content

[back][ext] feat: add rate limit to the bulk create route of the rate-later API#2064

Merged
GresilleSiffle merged 5 commits intorate-later-historyfrom
bulk-rate-later-ratelimit
Mar 21, 2025
Merged

[back][ext] feat: add rate limit to the bulk create route of the rate-later API#2064
GresilleSiffle merged 5 commits intorate-later-historyfrom
bulk-rate-later-ratelimit

Conversation

@GresilleSiffle
Copy link
Collaborator

@GresilleSiffle GresilleSiffle commented Mar 6, 2025

target PR: #2059


Description

  • limit the call to the rate-later bulk create API to 20 requests/day
  • handle the HTTP 429 status in the extension
  • display the number of new videos imported and the number of skipped videos

Checklist

  • I added the related issue(s) id in the related issues section (if any)
    • if not, delete the related issues section
  • I described my changes and my decisions in the PR description
  • I read the development guidelines of the CONTRIBUTING.md
  • The tests pass and have been updated if relevant
  • The code quality check pass

@GresilleSiffle GresilleSiffle changed the title Bulk rate later ratelimit [back][ext] feat: add rate limit to the bulk create route of the rate-later API Mar 6, 2025
@GresilleSiffle GresilleSiffle added Backend Back-end code of Tournesol Frontend Front-end code of Tournesol Extension Development of the browser extension and removed Frontend Front-end code of Tournesol labels Mar 6, 2025
@GresilleSiffle GresilleSiffle requested a review from sigmike March 6, 2025 17:07
@GresilleSiffle
Copy link
Collaborator Author

An idea for the future, it could be nice to replace the loading animation by a text (or something else) when the user reaches the maximum number of allowed calls to the API. Currently, when the animation disappears, the import status box becomes less visible, less eye catching than when it was loading.

capture

@sigmike
Copy link
Collaborator

sigmike commented Mar 9, 2025

Looks good to me. I have a few remarks:

  1. I don't know where the 20/day limit comes from, but couldn't we instead have a limit on a shorter time frame and make the extension wait a bit before it reaches the limit? This would allow users to keep this tab open if they want to import a large history (if it makes sense to do that). But it only makes sense if we can increase the average limit, because waiting 50 minutes doesn't seem reasonable.
  2. I agree that the changes happening when the limit is reached are barely noticeable. Maybe we could instead replace the texts and loader with something that looks like a Material UI warning Alert. The user would notice there's something new. And Maybe adding a fixed title to the box would make it look better.
  3. There may be too many numbers displayed. We can probably remove some.
  4. It's not a good time to do it, but adding React support to the extension would be make this kind of interface much easier to do.

@amatissart
Copy link
Member

  1. I don't know where the 20/day limit comes from, but couldn't we instead have a limit on a shorter time frame and make the extension wait a bit before it reaches the limit? This would allow users to keep this tab open if they want to import a large history (if it makes sense to do that). But it only makes sense if we can increase the average limit, because waiting 50 minutes doesn't seem reasonable.

Our quota of requests to the YouTube API is limited daily. So we need a safeguard to avoid the platform being blocked from using the YouTube API when users try to import a large number of videos.

One more improvement would be to store the ids of imported videos in the extension storage, so that they can be ignored on the next run, to let older videos in the history to be processed too. I think @GresilleSiffle plan to work on this tomorrow.

  1. It's not a good time to do it, but adding React support to the extension would be make this kind of interface much easier to do.

Yes we definitely see the limits of a "Vanilla" js approach on this kind of features. We were initially reluctant to add a framework that would require a separate build environment from the main "dev-env" to develop the extension. It may be easier now that "configure" scripts have been defined. A possible approach would also be to use "preact" which allows no-build workflows: https://preactjs.com/guide/v10/no-build-workflows

@GresilleSiffle
Copy link
Collaborator Author

I'm going to merge this branch in rate-later-history, and add a system to ignore previously imported videos. With see the vast majority of users will not be blocked by the daily limit.

@GresilleSiffle GresilleSiffle merged commit b1317dd into rate-later-history Mar 21, 2025
7 checks passed
@GresilleSiffle GresilleSiffle deleted the bulk-rate-later-ratelimit branch March 21, 2025 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Backend Back-end code of Tournesol Extension Development of the browser extension

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants