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

Move copay notifications back to concurrent approach #12938

Merged
merged 3 commits into from
Jun 9, 2023

Conversation

ScottyRJames
Copy link
Contributor

Summary

  • Window and bucket limiter caused sidekiq jobs that violated rate limit to fail

@ScottyRJames ScottyRJames requested review from a team as code owners June 8, 2023 21:28
@va-vfs-bot va-vfs-bot temporarily deployed to revert-to-concurrent-limiter/main/main June 8, 2023 21:41 Inactive
@va-vsp-bot va-vsp-bot requested a deployment to revert-to-concurrent-limiter/main/main June 8, 2023 21:59 In progress
@va-vsp-bot va-vsp-bot requested a deployment to revert-to-concurrent-limiter/main/main June 9, 2023 13:11 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to revert-to-concurrent-limiter/main/main June 9, 2023 13:16 Inactive
@va-vsp-bot va-vsp-bot requested a deployment to revert-to-concurrent-limiter/main/main June 9, 2023 16:46 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to revert-to-concurrent-limiter/main/main June 9, 2023 16:46 Inactive
Copy link
Contributor

@jeremy6d jeremy6d left a comment

Choose a reason for hiding this comment

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

I know this is a small change but in the future it would help us expedite if we had more explication in the description, at least a link to a ticket that directs this work and lays out the acceptance criteria. It would also give us a lot more confidence to get a review from one of your teammates or project-adjacent peers, as we don't have the context for the work you're doing like they do. Thanks!

@ScottyRJames
Copy link
Contributor Author

Thanks @jeremy6d, the other BE engineer on my team is out of office currently but I'll be sure to provide more context in future PRs

@ScottyRJames ScottyRJames merged commit 284253d into master Jun 9, 2023
@ScottyRJames ScottyRJames deleted the revert-to-concurrent-limiter branch June 9, 2023 17:38
ryan-mcneil pushed a commit that referenced this pull request Dec 11, 2023
* Move back to concurrent approach...

* Decrease to 4 workers
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.

4 participants