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.

Sign up for GitHub

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

Jump to bottom

Client-side rate limit interceptors (#520) #545

Closed
wants to merge 2 commits into from
Closed

Client-side rate limit interceptors (#520) #545

wants to merge 2 commits into from

Conversation

rahulkhairwar
Copy link
Contributor

Original Issue: here

@google-cla
Copy link

google-cla bot commented Mar 22, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@rahulkhairwar rahulkhairwar force-pushed the client-rate-limit-interceptors-520 branch from 7801504 to 29b4d63 Compare March 22, 2023 17:28
@rahulkhairwar rahulkhairwar mentioned this pull request Mar 22, 2023
Open
@rahulkhairwar rahulkhairwar force-pushed the client-rate-limit-interceptors-520 branch 3 times, most recently from 7b8ed47 to ccd4a38 Compare March 22, 2023 17:57
bwplotka
bwplotka approved these changes Mar 26, 2023
View reviewed changes
Copy link
Collaborator

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

Looks solid, thanks!

Want to rebase? Otherwise LGTM!

@rahulkhairwar rahulkhairwar force-pushed the client-rate-limit-interceptors-520 branch from ccd4a38 to 1effc9a Compare March 26, 2023 13:29
@rahulkhairwar
Copy link
Contributor Author

Hi @bwplotka !
I just rebased with the latest v2.
There's a failing check (lint issue, coming from: #544).
I believe once that is fixed, we can merge this PR too!
Lmk if you need anything from me, cheers! :)

@rahulkhairwar rahulkhairwar force-pushed the client-rate-limit-interceptors-520 branch from 2f2a531 to ae96d9c Compare July 4, 2023 09:15
@rahulkhairwar rahulkhairwar force-pushed the client-rate-limit-interceptors-520 branch from 5e4879d to 918411a Compare July 4, 2023 09:21
@rahulkhairwar
Copy link
Contributor Author

Hey @bwplotka , can we merge this PR? There was a pending linter check which was failing earlier, I fixed it (rebased with v2).

@bwplotka
Copy link
Collaborator

bwplotka commented Jul 4, 2023

Will check tomorrow, thanks!

bwplotka
bwplotka approved these changes Jul 5, 2023
View reviewed changes
Copy link
Collaborator

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

Nice, amazing. Thanks!

@bwplotka bwplotka changed the base branch from v2 to main July 5, 2023 13:35
@bwplotka bwplotka changed the base branch from main to v2 July 5, 2023 13:35
@bwplotka
Copy link
Collaborator

bwplotka commented Jul 5, 2023 •
edited
Loading

Hey, do you mind rebasing on "main"? Our old v2 is now main (sorry for confusion). Will remove v2 once all PRs will start to point to main.

@bwplotka bwplotka deleted the branch grpc-ecosystem:v2 July 5, 2023 13:40
@bwplotka bwplotka closed this Jul 5, 2023
@bwplotka
Copy link
Collaborator

bwplotka commented Jul 5, 2023

Closing as v2 branch got deleted. Feel free to rebase on main and reopen! 👍🏽

@rahulkhairwar
Copy link
Contributor Author

Yup, rebased with main and reopened (https://github.com/grpc-ecosystem/go-grpc-middleware/pull/599/files).

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

@bwplotka bwplotka bwplotka approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rahulkhairwar @bwplotka