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

Respect rate-limits during integration tests #941

Closed
raphael0202 opened this issue Jun 11, 2024 · 4 comments · Fixed by #943
Closed

Respect rate-limits during integration tests #941

raphael0202 opened this issue Jun 11, 2024 · 4 comments · Fixed by #943

Comments

@raphael0202
Copy link

Dart SDK integration tests seem to perform search queries (User-Agent: 'off-dart integration tests'), and these queries are above our rate limits (https://openfoodfacts.github.io/openfoodfacts-server/api/#rate-limits): 10req/s for search queries.
We're going to enforce the rate-limits soon using a rate-limiter, so these queries will fail in a near future.

We can either:

  • use the staging server
  • wait before each query
@monsieurtanuki
Copy link
Contributor

I'll have a look at it tomorrow.
Both solutions are indeed appropriate, and rather easy to implement.
Btw won't you ever have rate limit issues with the .net server?

@monsieurtanuki monsieurtanuki self-assigned this Jun 11, 2024
@raphael0202
Copy link
Author

raphael0202 commented Jun 12, 2024

Btw won't you ever have rate limit issues with the .net server?

The rate limits are currently not enforced on .net 👍

@monsieurtanuki
Copy link
Contributor

@raphael0202 Oops, I thought it was just about prices, which would have been much easier.

The thing is that the TEST env in unreliable. We cannot realistically switch our tests to TEST env as it is. Are there plans to make it more stable?

I've just run the tests in 3 minutes and there are:

  • at least 76 /api/v$version/product/$barcode/
  • at least 57 /cgi/search.pl

The requirements you refer to are:

  • 100 req/min for all read product queries (GET /api/v*/product requests or product page)
  • 10 req/min for all search queries (GET /api/v*/search or GET /cgi/search.pl requests)

That means the unit tests would take at least 57/10=5 more minutes, which is acceptable.
That said, each time I push code tests are run both in my github and off's. Which means that all pushes will fail the first time, and just rerunning them (only in off github) would pass.

@monsieurtanuki monsieurtanuki removed their assignment Jun 20, 2024
@raphael0202
Copy link
Author

raphael0202 commented Jun 24, 2024

That said, each time I push code tests are run both in my github and off's. Which means that all pushes will fail the first time, and just rerunning them (only in off github) would pass.

Rate-limits are per-ip, so as long as it's different IPs (which is very likely considering the tests are run on Github VMs), it should be fine.
We could also consider implementing an exception for some User-Agents (or some authenticated users), but it kind of introduce a vulnerability in the rate-limiter as it's easy to spot by looking at the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants