Skip to content

Conversation

m0ar
Copy link
Contributor

@m0ar m0ar commented Apr 7, 2025

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
Copy link
Contributor Author

m0ar commented Apr 15, 2025

Friendly ping @SgtPooki @achingbrain 🧁 👀

Copy link
Member

@SgtPooki SgtPooki left a comment

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

@@ -17,6 +17,7 @@ export const DEFAULT_TRUSTLESS_GATEWAYS = [

export interface HTTPGatewayRouterInit {
gateways?: Array<URL | string>
skipProviderShuffle?: boolean
Copy link
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?

Copy link
Member

@SgtPooki SgtPooki Apr 15, 2025

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
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
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
19 checks passed
@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