-
Notifications
You must be signed in to change notification settings - Fork 1.1k
PYTHON-2634 Only update pools for data-bearing servers #590
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
Conversation
Fixes a noisy OperationFailure: Authentication failed error. Do not attempt to create unneeded connections to arbiters, ghosts, hidden members, or unknown members.
prashantmital
left a comment
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 question about arbiters with auth - LGTM otherwise.
| arbiter = c._topology.get_server_by_address(('d', 4)) | ||
| self.assertFalse(arbiter.pool.sockets) | ||
|
|
||
| def test_direct_client_maintains_pool_to_arbiter(self): |
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.
In the directConnectio=True scenario, should we have any special handling for the case where authentication is enabled on the arbiter? Based on these docs:
the only way to log on to an arbiter with authorization active is to use the localhost exception.
it seems that we will never be able to populate a pool that connects to an arbiter with auth, or am I reading that wrong? What will the user-facing behavior be in this case?
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.
Yes, if a user wants to connect directly to an arbiter they will need to remove auth parameters.
|
Sigh, this change seems to make these two tests more flakey: And: I believe this is because the background thread now has to wait for the SDAM state to be discovered before attempting to populate the pool but I am very surprised to see that this wouldn't happen within the test's 10 second timeout. Perhaps there is too much thread contention and one of the monitors is being starved for 10 seconds? |
…which will not have thier pools updated
Fixes a noisy OperationFailure: Authentication failed error. Do not attempt to create unneeded connections to arbiters, ghosts, hidden members, or unknown members. (cherry picked from commit 4c7718e) Conflicts: pymongo/topology.py test/test_client.py test/test_cmap.py
Fixes a noisy OperationFailure: Authentication failed error. Do not attempt to create unneeded connections to arbiters, ghosts, hidden members, or unknown members. (cherry picked from commit 4c7718e) Conflicts: pymongo/topology.py test/test_client.py test/test_cmap.py
Fixes a noisy OperationFailure: Authentication failed error.
Do not attempt to create unneeded connections to arbiters, ghosts,
hidden members, or unknown members.