-
Notifications
You must be signed in to change notification settings - Fork 168
BM-1908: Add support for FallbackLayer to boundless sdk #1324
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
base: main
Are you sure you want to change the base?
Conversation
| let active_count = | ||
| std::num::NonZeroUsize::new(transports.len()).unwrap_or(std::num::NonZeroUsize::MIN); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should just ignore the case of no transports being selected? This feels like we would be opening up to issues where there are 0 transports, but the fallback layer assumes there is one? Given that we are only calling this if there are values, should this not just pass back an error in this case?
| // Add the primary RPC URL if set | ||
| if let Some(ref rpc_url) = self.rpc_url { | ||
| all_urls.push(rpc_url.clone()); | ||
| } | ||
|
|
||
| // Add any additional URLs from rpc_urls | ||
| all_urls.extend(self.rpc_urls.clone()); | ||
|
|
||
| Ok(all_urls) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like erroring if all_urls is empty here might be helpful with a message that indicates you need to set either or both variables?
Support FallbackLayer in boundless SDK and in broker. This allows configuring multiple RPCs and trying all of them.
Enables it for one of the two single instance provers we operate, so we can monitor and compare perf to the other one