-
Notifications
You must be signed in to change notification settings - Fork 180
RUST-950 Enable connection to a load balancer #415
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
RUST-950 Enable connection to a load balancer #415
Conversation
// TODO RUST-653 Remove this when load balancer work is ready for release. | ||
#[builder(default, setter(skip))] | ||
#[serde(skip)] | ||
pub(crate) allow_load_balanced: bool, |
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.
This awkward double-flag construction allows load_balanced
to be populated by the URI/TXT parsing as it will be on release, but prevents that from being used outside of tests that have access to set allow_load_balanced
to be true.
4f2644a
to
afd1b72
Compare
src/srv.rs
Outdated
@@ -189,11 +191,28 @@ impl SrvResolver { | |||
"replicaset" => { | |||
config.replica_set = Some(parts[1].into()); | |||
} | |||
"loadBalanced" => { |
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.
This should be lowercase. See Line 187.
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.
Good catch, thank you!
This has broken all tests with auth enabled; converted to draft while I track that down. |
Ah, found it; I'd broken |
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.
Looking good! Just have a few minor suggestions / questions.
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.
One tiny nitpick otherwise LGTM!
c340c54
to
78bfb11
Compare
RUST-950
This implements the connection establishment portion of load balancer support, i.e.:
loadBalanced
URI/TXT optionserviceId
connection pool trackingThe work here happened to also encompass RUST-955.