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

fix: revert concurrency in Modules #4730

Merged
merged 3 commits into from
Oct 22, 2024
Merged

fix: revert concurrency in Modules #4730

merged 3 commits into from
Oct 22, 2024

Conversation

ltyu
Copy link
Contributor

@ltyu ltyu commented Oct 22, 2024 •
edited
Loading

Description

Reverts #4720

Reasons:

  • Many of the public RPCs don't allow concurrency reads. The ones that do don't tell us how much they allow so no point in defaulting to some assumed number (20)
  • retryAsync doesn't do anything here as the SmartProvider knows what to do if a provider fails (retries with cooldown). SmartProvider has it's own retryAsync
  • Concurrency is configurable in the metadata. so configure for each provider, as intended.
Screenshot 2024-10-22 at 4 11 02 PM

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Oct 22, 2024

⚠️ No Changeset found

Latest commit: d63351d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@codecov Codecov
Copy link

codecov bot commented Oct 22, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.74%. Comparing base (2934195) to head (d63351d).
Report is 1 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #4730   +/-   ##
=======================================
  Coverage   73.74%   73.74%           
=======================================
  Files         100      100           
  Lines        1436     1436           
  Branches      187      187           
=======================================
  Hits         1059     1059           
  Misses        356      356           
  Partials       21       21
Components Coverage Δ
core 84.61% <ø> (ø)
hooks 75.71% <ø> (ø)
isms 77.58% <ø> (ø)
token 88.75% <ø> (ø)
middlewares 77.39% <ø> (ø)

@yorhodes
Copy link
Member

Reason: discord.com/channels/935678348330434570/1297972601536385034/1298373790996107285

this link doesnt want to load for me for whatever reason
lets try to keep PR descriptions with markdown text as much as possible (to preserve git log etc)

@ltyu ltyu changed the title fix: revert partial concurrency fix: revert concurrency in Modules Oct 22, 2024
@ltyu ltyu enabled auto-merge October 22, 2024 20:14
@ltyu ltyu added this pull request to the merge queue Oct 22, 2024
Merged via the queue into main with commit cd7a263 Oct 22, 2024
33 checks passed
@ltyu ltyu deleted the ltyu/revert-partial-concurrency branch October 22, 2024 21:54
tiendn pushed a commit to tiendn/hyperlane-monorepo that referenced this pull request Oct 25, 2024
@ltyu @tiendn
fix: revert concurrency in Modules (hyperlane-xyz#4730)
c43c9ec 
### Description
Reverts hyperlane-xyz#4720

Reasons:
- Many of the public RPCs don't allow concurrency reads. The ones that
do don't tell us how much they allow so no point in defaulting to some
assumed number (20)
- retryAsync doesn't do anything here as the SmartProvider knows what to
do if a provider fails (retries with cooldown). SmartProvider has it's
own retryAsync
- Concurrency is configurable in the metadata. so configure for each
provider, as intended.
<img width="1233" alt="Screenshot 2024-10-22 at 4 11 02 PM"
src="https://github.com/user-attachments/assets/853cb224-c8c1-4a80-bf7e-8469ab3cc045">
tiendn pushed a commit to tiendn/hyperlane-monorepo that referenced this pull request Oct 25, 2024
@ltyu @tiendn
fix: revert concurrency in Modules (hyperlane-xyz#4730)
49e1d69 
### Description
Reverts hyperlane-xyz#4720

Reasons:
- Many of the public RPCs don't allow concurrency reads. The ones that
do don't tell us how much they allow so no point in defaulting to some
assumed number (20)
- retryAsync doesn't do anything here as the SmartProvider knows what to
do if a provider fails (retries with cooldown). SmartProvider has it's
own retryAsync
- Concurrency is configurable in the metadata. so configure for each
provider, as intended.
<img width="1233" alt="Screenshot 2024-10-22 at 4 11 02 PM"
src="https://github.com/user-attachments/assets/853cb224-c8c1-4a80-bf7e-8469ab3cc045">
tiendn pushed a commit to tiendn/hyperlane-monorepo that referenced this pull request Oct 25, 2024
@ltyu @tiendn
fix: revert concurrency in Modules (hyperlane-xyz#4730)
16701f4 
### Description
Reverts hyperlane-xyz#4720

Reasons:
- Many of the public RPCs don't allow concurrency reads. The ones that
do don't tell us how much they allow so no point in defaulting to some
assumed number (20)
- retryAsync doesn't do anything here as the SmartProvider knows what to
do if a provider fails (retries with cooldown). SmartProvider has it's
own retryAsync
- Concurrency is configurable in the metadata. so configure for each
provider, as intended.
<img width="1233" alt="Screenshot 2024-10-22 at 4 11 02 PM"
src="https://github.com/user-attachments/assets/853cb224-c8c1-4a80-bf7e-8469ab3cc045">
tiendn pushed a commit to tiendn/hyperlane-monorepo that referenced this pull request Oct 25, 2024
@ltyu @tiendn
fix: revert concurrency in Modules (hyperlane-xyz#4730)
d5f8365 
### Description
Reverts hyperlane-xyz#4720

Reasons:
- Many of the public RPCs don't allow concurrency reads. The ones that
do don't tell us how much they allow so no point in defaulting to some
assumed number (20)
- retryAsync doesn't do anything here as the SmartProvider knows what to
do if a provider fails (retries with cooldown). SmartProvider has it's
own retryAsync
- Concurrency is configurable in the metadata. so configure for each
provider, as intended.
<img width="1233" alt="Screenshot 2024-10-22 at 4 11 02 PM"
src="https://github.com/user-attachments/assets/853cb224-c8c1-4a80-bf7e-8469ab3cc045">
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yorhodes yorhodes yorhodes approved these changes

@jmrossy jmrossy Awaiting requested review from jmrossy jmrossy is a code owner

@paulbalaji paulbalaji Awaiting requested review from paulbalaji paulbalaji is a code owner

Assignees
No one assigned
Labels
None yet
Projects
Hyperlane Tasks
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ltyu @yorhodes