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

curl: add --rate to set max request rate per time unit #8671

Closed
wants to merge 1 commit into from

Conversation

bagder
Copy link
Member

@bagder bagder commented Apr 4, 2022

--rate "12/m" - for 12 per minute or
--rate "5/h" - for 5 per hour

This option is only for serial transfers, when curl is told to do more than one request.

  • man page section added

@jay
Copy link
Member

@jay jay commented Apr 4, 2022

what's with the slash? how about 12s 5m 1h. or if you're worried 5m will be confused with limit-rate then how about --request-rate 5min

@jzakrzewski
Copy link
Contributor

@jzakrzewski jzakrzewski commented Apr 4, 2022

The slash looks better IMHO. Consider what happens when you read it: "12s" is "12 seconds", where "12/s" is "12 per second" kinda without thinking. So without the slash "12s" would be like "once every 12 seconds" to me, which is not what the option does, right?

@bagder
Copy link
Member Author

@bagder bagder commented Apr 4, 2022

Yes, my thinking was exactly what @jzakrzewski said here.

@jay
Copy link
Member

@jay jay commented Apr 5, 2022

So without the slash "12s" would be like "once every 12 seconds" to me, which is not what the option does, right?

Yes you're right. Though I still like --request-rate. Why is the option limited to serial transfers?

@bagder
Copy link
Member Author

@bagder bagder commented Apr 5, 2022

Though I still like --request-rate.

I was considering that, but then I wasn't sure if request is really the best term when speaking other protocols than HTTP as well - it would perhaps be better with --transfer-rate but that also didn't feel appropriate so I got a little lost. 😁

Why is the option limited to serial transfers?

How would it work for parallel? I couldn't think of how it should work so I left that out until we can figure that out.

@777777dl

This comment was marked as disruptive content.

bagder added a commit that referenced this issue Apr 6, 2022
--rate "12/m" - for 12 per minute or
--rate "5/h" - for 5 per hour

Removed from TODO

Closes #8671
@Whissi
Copy link

@Whissi Whissi commented Apr 8, 2022

Hi!

Your new parameter still requires manual interaction, i.e. user has to know and set proper rate limiting on their own.

Any chance to implement support for X-Ratelimit-* headers as described in https://tools.ietf.org/id/draft-polli-ratelimit-headers-02.html which I think Github, Twitter and some others have already implemented?

@bagder
Copy link
Member Author

@bagder bagder commented Apr 8, 2022

Correct (and note that you link to an outdated version of the draft). I'm going for simplicity to start with.

If we want curl to adapt to what the response headers' say, then I think we should make that an explicit instruction for the --rate option. Maybe --rate as-indiciated or something, and we probably even need to have the option as "use this rate or whatever rate the site might say".

Something to keep in mind when thinking of letting it adapt to what the server says: as this rate frequency feature is implemented now, it works independently of which servers curl speaks to. It means that you could do curl $host1 $host2 $host3 and they could very well each return a different set of ratelimit headers...

bagder added a commit that referenced this issue May 12, 2022
--rate "12/m" - for 12 per minute or
--rate "5/h" - for 5 per hour

Removed from TODO

Closes #8671
--rate "12/m" - for 12 per minute or
--rate "5/h" - for 5 per hour

Removed from TODO

Closes #8671
@bagder bagder closed this in 8f48b5d May 23, 2022
@bagder bagder deleted the bagder/curl-rate branch May 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants