Skip to content

feat: add provider shuffle option to HTTPGatewayRouter - #772

Merged
SgtPooki merged 2 commits into
ipfs:mainfrom
m0ar:m0ar/provider-shuffle-opt-out
Apr 16, 2025
Merged

feat: add provider shuffle option to HTTPGatewayRouter#772
SgtPooki merged 2 commits into
ipfs:mainfrom
m0ar:m0ar/provider-shuffle-opt-out

Conversation

@m0ar

@m0ar m0ar commented Apr 7, 2025

Copy link
Copy Markdown
Contributor

feat: allow opting out of provider shuffle in HTTPGatewayRouter

See #771 for motivation.

Description

For use cases where gateway lookup order is important, this PR allows passing a new config param to HTTPGatewayRouter which opts out of the shuffling which happens on each call to findProviders.

Notes & open questions

  • The new param is optional
  • Default value preserves the original behavior

I.e., the feature is backward-compatible.

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if necessary (this includes comments as well)
  • I have added tests that prove my fix is effective or that my feature works

@m0ar

m0ar commented Apr 15, 2025

Copy link
Copy Markdown
Contributor Author

Friendly ping @SgtPooki @achingbrain 🧁 👀

@SgtPooki SgtPooki left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

code looks fine to me.. i think the option could have a better name


export interface HTTPGatewayRouterInit {
gateways?: Array<URL | string>
skipProviderShuffle?: boolean

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

maybe something more explicit like keepOrder would be better?

@SgtPooki SgtPooki Apr 15, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

also, we should probably add a jsdoc with @default tag..

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Or even more concise - ‘shuffle’ (default: true)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@SgtPooki @achingbrain Thanks for the review!

Just pushed a commit changing it to shuffle, flipped the corresponding logic, and added a jsdoc comment with a @default tag ✔️

@SgtPooki SgtPooki changed the title feat: allow opting out of provider shuffle in HTTPGatewayRouter feat: add provider shuffle option to HTTPGatewayRouter Apr 16, 2025
@SgtPooki
SgtPooki merged commit daaa511 into ipfs:main Apr 16, 2025
@achingbrain achingbrain mentioned this pull request Apr 16, 2025
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.

3 participants