Mongo read preference #393
Conversation
|
this looks great and inline with the general code style I took a cursory look at the failing tests and will try to reproduce locally. |
|
Excellent. I'll try and figure out what is going wrong with the tests too - although not had much luck so far. |
|
@SamBartrum ok, I tracked down the issue, the tests use a different session to check doc counts and such. You need to add |
Codecov Report
@@ Coverage Diff @@
## master #393 +/- ##
==========================================
+ Coverage 84.42% 84.57% +0.14%
==========================================
Files 59 59
Lines 4046 4071 +25
==========================================
+ Hits 3416 3443 +27
+ Misses 468 467 -1
+ Partials 162 161 -1
Continue to review full report at Codecov.
|
adaptor/mongodb/client.go
Outdated
| c.readPreference = DefaultReadPreference | ||
| return nil | ||
| } | ||
| switch read_preference { |
There was a problem hiding this comment.
I'm thinking it'd be nice to do a strings.ToLower(read_preference) here so the preference is insensitive.
| readPreference: 2, | ||
| }, | ||
| nil, | ||
| }, |
There was a problem hiding this comment.
please add test cases to cover all possible preferences
client/error.go
Outdated
| @@ -49,3 +49,12 @@ func (e VersionError) Error() string { | |||
| } | |||
| return fmt.Sprintf("%s running %s, %s", e.URI, e.V, e.Err) | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
this should be in adaptor/mongodb/client.go (below here) since it's specific to mongodb
|
LGTM, if you could rebase from master and preferably squash the commits, I'll get this merged in |
SamBartrum
force-pushed
the
mongo_read_preference
branch
from
ac11ebe to
60b614b
Compare
… squashed commits) Squashed commits: [60b614b] Move mongo specific error to mongodb/client. Add more tests to cover all read preferences. Allow for case insensitive read preference to be set. (+5 squashed commits) Squashed commits: [381efce] Fix failing tests [82c48da] Unit tests for mongo read preference setting. Fixed other unit tests in light of the new default read preference. [11e7ed9] Update readme to show read preference in the mongo config. [ae4c94e] Remove mgo specific read preferences. [b636058] Can now set mongo read preference in config, defaults to Primary and raises an error if the read preference is not known. [677afaf] Initial commit. Starting to expose option to set mongo read preference.
SamBartrum
force-pushed
the
mongo_read_preference
branch
from
60b614b to
3306ac0
Compare
|
Just added a last minute test for the InvalidReadPreferenceError and fixed a typo in a test. It should now be rebased from master and squashed into a single commit. Good to go? |
A potential fix relating to this issue: #392
Expose the ability to set the mongo read preference within the connection config.
(This is my first time writing any go, so go easy on me!)
Required for all PRs: