[GODRIVER-1498] Refactor URI parsing to use pointers Created: 27/Feb/20 Updated: 08/Jan/24 |
|
| Status: | Backlog |
| Project: | Go Driver |
| Component/s: | Options & Configuration |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | David Bartley | Assignee: | Unassigned |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| Epic Link: | GODRIVER-1588 | ||||||||
| Quarter: | FY25Q1 | ||||||||
| Documentation Changes Summary: | 1. What would you like to communicate to the user about this feature? |
||||||||
| Description |
|
New Description: The URI parser uses two variables per option: one for the value and one for determining if the option is set. This code that uses the parser is inconsistent. Sometimes, it checks the *Set variant but for some boolean options that are only useful if set to true (e.g. SSL), it will directly check cs.SSL rather than cs.SSLSet && cs.SSL. Let's refactor this to use pointers like the rest of our options code for consistency.
Previous Description: Pretty minor, but ConnString.SSLSet is never read by anything. I believe https://github.com/mongodb/mongo-go-driver/blob/a96725e2d7cb39d0c31f7f6605305ad3f92787c1/x/mongo/driver/topology/topology_options.go#L112 should actually be "if cs.SSLSet && cs.SSL {". |
| Comments |
| Comment by Divjot Arora (Inactive) [ 27/Feb/20 ] |
|
bartle Thanks for the quick reply. I've edited the ticket title and description to reflect the work that should be done. |
| Comment by David Bartley [ 27/Feb/20 ] |
|
Yes, that seems like a great approach. |
| Comment by Divjot Arora (Inactive) [ 27/Feb/20 ] |
|
bartle I'd like to do a larger piece of work and re-write the connstring parser to use pointers rather than having Option and OptionSet for each URI option. With the way it is now, it's easy to be inconsistent in code that uses the connection string. For options that default to false but are only useful when set to true (like SSL/SSLInsecure), we often don't check *Set, which isn't incorrect, but it is inconsistent.
Do you mind if I re-purpose this ticket into an improvement for the work I described? |
| Comment by David Bartley [ 27/Feb/20 ] |
|
SSLInsecureSet is similarly never read. |
| Comment by David Bartley [ 27/Feb/20 ] |