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

add option to not shuffle providers #588

Closed
wants to merge 1 commit into from
Closed

add option to not shuffle providers #588

wants to merge 1 commit into from

Conversation

NoahZinsmeister
Copy link

While looking through the v5 implementation of the FallbackProvider, I thought it might be nice to have an option not to shuffle providers. This option could be turned off by default (i.e. shuffle unless specifically told not to) as it is in this PR, but it would be a nice option in situations where developers want to call providers in a fixed order, only falling back to higher index providers in cases of errors.

My other thought was to modify the shuffled function to accept weights and do a weighted shuffle. In some ways this is more robust, but is also a bit harder to reason about (weighted shuffles seem strange). Let me know if you'd be more amenable to this approach, though.

@NoahZinsmeister
Copy link
Author

One other thought - it might be nice to be able to specify a timeout for requests (maybe per-provider?), so that hanging requests trigger errors. Would you be interested in seeing something like this implemented? I'd be happy to do the work if you're likely to merge :)

@ricmoo ricmoo added the discussion Questions, feedback and general information. label Sep 28, 2019
ricmoo added a commit that referenced this pull request Jan 19, 2020
… are only approximate and supports per-provider priorities (#635, #588).
@ricmoo
Copy link
Member

ricmoo commented Jan 19, 2020

I think this should help add an equivalent of this feature in the newest version.

Basically, only providers within the same "priority" are shuffled, so to ensure the order different priorities can be given to each.

Please see the docs.

Let me know what you think. Thanks! :)

@NoahZinsmeister
Copy link
Author

The priority feature looks great, and definitely addresses this issue! Thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Questions, feedback and general information.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants