-
Notifications
You must be signed in to change notification settings - Fork 455
CDRIVER-4056 Send LB in handshake and reject non-LB connections #828
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
CDRIVER-4056 Send LB in handshake and reject non-LB connections #828
Conversation
2fffa97
to
8cc94b1
Compare
8cc94b1
to
b790410
Compare
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 looks good to me! I'm going a little bit on faith that the tests test what they're supposed to test (looks like they do)...
*/ | ||
bool | ||
mongoc_server_description_service_id ( | ||
const mongoc_server_description_t *description, bson_oid_t *oid); |
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.
nit: It's probably overly-pedantic, as basically nobody writes this, but did you mean:
bool
mongoc_server_description_service_id (
const mongoc_server_description_t * const description, bson_oid_t *oid);
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 did not. I do not think a const pointer adds a guarantee from the caller's perspective.
If mongoc_server_description_service_id
promises not to change what description points to, a caller could not observe it.
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.
+1. And I don't think it really buys anything, either.
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.
LGTM, just a nit.
src/libmongoc/doc/errors.rst
Outdated
@@ -31,6 +31,8 @@ Many C Driver functions report errors by returning ``false`` or -1 and filling o | |||
+-----------------------------------------+---------------------------------------------------------------------------------------------------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+ | |||
| | ``MONGOC_ERROR_CLIENT_INVALID_ENCRYPTION_STATE`` | Failure related to Client-Side Field Level Encryption. | | |||
+-----------------------------------------+---------------------------------------------------------------------------------------------------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+ | |||
| | ``MONGOC_ERROR_CLIENT_INVALID_LOAD_BALANCER`` | You attempted to connect to a MongoDB server behind a load balancer, but the server does not advertise load balanced support. | |
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.
British spelling for advertise
but I assume this error message was copied from somewhere.
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.
TIL I've been using the British spelling. Updated.
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.
+1 LGTM
Uh oh!
There was an error while loading. Please reload this page.