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

Added DirectRateLimiter::until_n_ready{,_with_jitter} #10

Merged
merged 1 commit into from
Feb 2, 2020

Conversation

jean-airoldie
Copy link
Contributor

Allows the user to wait asynchronously for n cells to be available.

Closes #9.

@jean-airoldie
Copy link
Contributor Author

Blocked by #11.

Copy link
Collaborator

@antifuchs antifuchs left a comment

Choose a reason for hiding this comment

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

Thanks so much for the contribution! This looks great overall, and I've left some comments. Once #11 goes in, we should be OK to merge this in, with the suggested changes.

src/state/direct/future.rs Outdated Show resolved Hide resolved
src/state/direct/future.rs Outdated Show resolved Hide resolved
Allows the user to wait asynchronously for n cells to be available.
@jean-airoldie
Copy link
Contributor Author

Alright so here is my two cents:

For the sake of consistency between the check_all method and the until_ready for multiples cells I think both should either be named:

  • check_all & until_all_ready
  • check_n & until_n_ready

Personally I think that the _n version is clearer because the _all version seems to imply that we wait for all the capacity to be available.

@jean-airoldie
Copy link
Contributor Author

Btw cool crate. Very well documented as well.

@jean-airoldie jean-airoldie changed the title WIP: Added DirectRateLimiter::until_n_ready{,_with_jitter} Added DirectRateLimiter::until_n_ready{,_with_jitter} Feb 2, 2020
@antifuchs
Copy link
Collaborator

You make a good point about the naming - with the _all convention, I'd hoped to provide a check_any method as well, where the request is allowed to proceed if requested<= n > 0 cells are allowed, but that seems very confusing and probably doesn't provide as big a benefit as I'd hoped (plus, how much should this reserve? It would be confusing).

I'm ok with the _n naming on until, but the question about consistency remains. I'll rename check_all to check_n in a follow-up change to this.

Also, thanks so much for the kind words! I'm super glad you find it useful & that you like the documentation!

@antifuchs
Copy link
Collaborator

bors r+

bors bot added a commit that referenced this pull request Feb 2, 2020
10: Added DirectRateLimiter::until_n_ready{,_with_jitter} r=antifuchs a=jean-airoldie

Allows the user to wait asynchronously for n cells to be available.

Closes #9.

Co-authored-by: jean-airoldie <25088801+jean-airoldie@users.noreply.github.com>
@bors
Copy link
Contributor

bors bot commented Feb 2, 2020

Build succeeded

@bors bors bot merged commit a48514e into boinkor-net:master Feb 2, 2020
@jean-airoldie jean-airoldie deleted the until_n_ready branch February 3, 2020 13:20
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.

Support waiting for an abitrary weight
2 participants