[SERVER-47750] ismaster server response now returns a string instead of a boolean Created: 24/Apr/20 Updated: 29/Oct/23 Resolved: 24/Apr/20 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 4.7.0 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Matt Broadstone | Assignee: | Pavithra Vetriselvan |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||
| Operating System: | ALL | ||||||||
| Sprint: | Repl 2020-05-04 | ||||||||
| Participants: | |||||||||
| Description |
|
Recently the value type for the ismaster property of an ismaster command response changed from a boolean to a string. I believe this change is responsible. |
| Comments |
| Comment by Matt Broadstone [ 27/Apr/20 ] |
|
thanks pavithra.vetriselvan! |
| Comment by Pavithra Vetriselvan [ 27/Apr/20 ] |
|
matt.broadstone, I ended up filing |
| Comment by Matt Broadstone [ 25/Apr/20 ] |
|
Whatever you think is best. I would just like us to have coverage around this considering it it's critical "public API" between server and drivers. |
| Comment by Pavithra Vetriselvan [ 24/Apr/20 ] |
|
The unittests in |
| Comment by Pavithra Vetriselvan [ 24/Apr/20 ] |
|
Agreed, I checked my changes locally by adding a check to awaitable_ismaster.js, which seems to validate the isMaster request. It was more of a sanity check than a full fledged addition. I didn't end up submitting this for review because I thought it'd be more valuable to add a test that validates the types of all isMaster response fields, as you mentioned offline. I think ideally, we would have used the IDL for MongosIsMasterResponse, but I think that's currently out of scope (could definitely be an additional code cleanup ticket though!). We planned on adding unittests in |
| Comment by Matt Broadstone [ 24/Apr/20 ] |
|
Hey y'all, I see there weren't any tests added for this change. Seems like it would go a long way to prevent these types of issues in the future. |
| Comment by Githook User [ 24/Apr/20 ] |
|
Author: {'name': 'Pavi Vetriselvan', 'email': 'pvselvan@umich.edu', 'username': 'pvselvan'}Message: |