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

Long-term rate limits for package uploads. #8063

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

isoos
Copy link
Collaborator

@isoos isoos commented Sep 18, 2024

  • Create an allow-list of packages that can publish more than 1000 versions #8053
  • The current rate limit implementation relies on reading all of the audit log records from the datastore and keeping them in memory. While we could read a full year's worth of logs into the memory, it seemed better to provide a separate path for package uploads, as we do have the timestamps at the time of rate limit checks.
  • I've added additional checks that (a) we shouldn't specify long-term rate limits for other actions and (b) if we do and we don't provide the timestamps, it should fail.
  • In order to reduce the surprises, I think we could start introducing the weekly limit, and then shortly after that the monthly, while the quarterly limit should be added only later this year, and the yearly in the next year. That way packages that would be over the specific limit have time to adopt.
  • Giving a feedback of the remaining limit may come in a subsequent PR.

@isoos isoos requested review from jonasfj and sigurdm September 18, 2024 16:03
Comment on lines +79 to +81
# monthly: 32 # ~ 8 versions/week
# quarterly: 50 # ~ 4 versions/week
# yearly: 100 # ~ 2 versions/week
Copy link
Member

Choose a reason for hiding this comment

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

I think the problem with monthly, quarterly and yearly is that these limits are so long that we need a mechanism to override them.

If you publish ~50 versions per quarter, then suddenly you can't publish for 6 months.
I don't think that's viable.


I do think that 20 publications per week seems like a reasonable limit. And having to wait 5 days is not an entirely unreasonable ask (in such scenarios, hopefully there are few).

Copy link
Member

Choose a reason for hiding this comment

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

I think a client-side warning or error is probably better:
dart-lang/pub#4392

We just want a soft limit, really.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If you publish ~50 versions per quarter, then suddenly you can't publish for 6 months.

The daily, weekly and monthly limit still apply, and it is unlikely that you would trigger only the quarterly and nothing before. However, if you do hit this scenario, your events are stretched out over the quarter. As the window is rolling, your prior events will move out of it, you won't need to wait for the entire quarter to get more quota.

Maybe the events could have an exponential decay in them. E.g. after a month an event may count as only 0.5 event, and as such, the emergency quota gets freed up a bit faster?

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