Skip to content

More ratelimit fixes#177

Merged
amCap1712 merged 2 commits intomainfrom
more-ratelimit-fixes
Aug 7, 2025
Merged

More ratelimit fixes#177
amCap1712 merged 2 commits intomainfrom
more-ratelimit-fixes

Conversation

@amCap1712
Copy link
Copy Markdown
Member

  1. Fix ZeroDivisionError.
  2. Pass auth token to relevant api calls in LB radio patch.

@amCap1712 amCap1712 requested a review from mayhem August 6, 2025 19:49
@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 6, 2025

Test Results

  4 files  ±0    4 suites  ±0   4s ⏱️ ±0s
 50 tests ±0   50 ✅ ±0  0 💤 ±0  0 ❌ ±0 
200 runs  ±0  200 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 1b9ba31. ± Comparison against base commit abd9965.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 6, 2025

Package Line Rate Complexity Health
. 33% 0
content_resolver 0% 0
content_resolver.formats 6% 0
content_resolver.model 0% 0
external 39% 0
listenbrainz 46% 0
musicbrainz 72% 0
patches 27% 0
patches.lb_radio_classes 0% 0
tools 20% 0
. 33% 0
content_resolver 0% 0
content_resolver.formats 6% 0
content_resolver.model 0% 0
external 39% 0
listenbrainz 46% 0
musicbrainz 72% 0
patches 27% 0
patches.lb_radio_classes 0% 0
tools 20% 0
. 33% 0
content_resolver 0% 0
content_resolver.formats 6% 0
content_resolver.model 0% 0
external 39% 0
listenbrainz 46% 0
musicbrainz 72% 0
patches 27% 0
patches.lb_radio_classes 0% 0
tools 20% 0
. 33% 0
content_resolver 0% 0
content_resolver.formats 6% 0
content_resolver.model 0% 0
external 39% 0
listenbrainz 46% 0
musicbrainz 72% 0
patches 27% 0
patches.lb_radio_classes 0% 0
tools 20% 0
Summary 21% (4303 / 20182) 0

@amCap1712
Copy link
Copy Markdown
Member Author

If we merge this change, we also need to update this line in LB to pass a token to troi to make use of the better ratelimit.
https://github.com/metabrainz/listenbrainz-server/blob/e10e33bb5abb01282dea60ada70ca5564e520eee/listenbrainz/webserver/views/explore_api.py#L184

However, now I wonder whether we should do that as it would let people DDoS pretty easily? I think instead of passing a whitelisted token always there we should forward the token received in the API call to that endpoint to troi. So the higher ratelimit would only be allowed if we invoked the endpoint.

@mayhem
Copy link
Copy Markdown
Member

mayhem commented Aug 7, 2025

If we merge this change, we also need to update this line in LB to pass a token to troi to make use of the better ratelimit. https://github.com/metabrainz/listenbrainz-server/blob/e10e33bb5abb01282dea60ada70ca5564e520eee/listenbrainz/webserver/views/explore_api.py#L184

However, now I wonder whether we should do that as it would let people DDoS pretty easily? I think instead of passing a whitelisted token always there we should forward the token received in the API call to that endpoint to troi. So the higher ratelimit would only be allowed if we invoked the endpoint.

Very good thinking, lets do that.

Copy link
Copy Markdown
Member

@mayhem mayhem left a comment

Choose a reason for hiding this comment

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

Who could've thought that an API first toolkit could run into so many rate limitings issues? 🤷

@amCap1712 amCap1712 merged commit 8f4ddd0 into main Aug 7, 2025
6 checks passed
@amCap1712 amCap1712 deleted the more-ratelimit-fixes branch August 7, 2025 12:45
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.

2 participants