-
Notifications
You must be signed in to change notification settings - Fork 532
RUBY-3743 Finish implementing spec changes from DRIVERS-1519 #2973
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
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.
Pull request overview
This PR implements spec changes from DRIVERS-1519 for SRV monitoring with srvMaxHosts support. The changes ensure that the srv_max_hosts parameter is properly passed through the monitoring system and that SRV resolution failures are handled according to specification requirements.
Key Changes:
- Modified SRV resolver to not raise errors on invalid domain verification per spec
- Updated SRV monitor and URI parser to pass through
srv_max_hostsandsrv_service_nameoptions from both URI and client options - Added comprehensive test coverage for
srv_max_hostsfunctionality in the SRV monitor
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| spec/mongo/srv/monitor_spec.rb | Adds test cases verifying that srv_max_hosts limits cluster hosts and is correctly passed to the resolver |
| lib/mongo/uri/srv_protocol.rb | Changes resolver to use raise_on_invalid: false and adds fallback logic for srv_service_name and srv_max_hosts options |
| lib/mongo/srv/monitor.rb | Updates scan! method to pass srv_service_name and srv_max_hosts options to resolver |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lib/mongo/uri/srv_protocol.rb
Outdated
| hostname, | ||
| uri_options[:srv_service_name] || options[:srv_service_name], |
Copilot
AI
Jan 5, 2026
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.
Removed trailing whitespace from lines 155 and 156.
| hostname, | |
| uri_options[:srv_service_name] || options[:srv_service_name], | |
| hostname, | |
| uri_options[:srv_service_name] || options[:srv_service_name], |
jamis
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.
Looks great! Optionally: cleaning up that line I mentioned in srv/resolver.rb, but that is clearly outside the scope of this ticket, so I'll leave that up to you.
|
Locally tested these changes using the following script: require 'logger'
require 'debug'
require 'bundler/inline'
gemfile do
source 'https://rubygems.org'
# gem 'mongoid', '~> 7.5.4'
gem 'mongo', path: __dir__
end
Mongo::Logger.logger = Logger.new($stdout)
Mongo::Logger.logger.level = Logger::INFO
URL = 'mongodb+srv://test1.test.build.10gen.cc/?tls=false'
client = Mongo::Client.new(URL, connect_timeout: 5, server_selection_timeout: 10, socket_timeout: 5, srv_max_hosts: 1)
puts "==== size: #{client.cluster.addresses.size}"
sleep(5)
puts "==== size: #{client.cluster.addresses.size}"
URL_WITH_MAX_HOSTS = 'mongodb+srv://test1.test.build.10gen.cc/?tls=false&srvMaxHosts=1'
client2 = Mongo::Client.new(URL_WITH_MAX_HOSTS, connect_timeout: 5, server_selection_timeout: 10, socket_timeout: 5)
puts "==== size: #{client2.cluster.addresses.size}"
sleep(5)
puts "==== size: #{client2.cluster.addresses.size}"Output: |
srv_max_hostsoptionsrv_max_hostsoption in the Mongo client builder from being usedraise_on_invalidin the SRV resolver to false to adhere to spec requirements