Futzing with read preference #3963
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3963 +/- ##
==========================================
- Coverage 94.04% 94.03% -0.01%
==========================================
Files 129 129
Lines 9214 9243 +29
==========================================
+ Hits 8665 8692 +27
- Misses 549 551 +2
Continue to review full report at Codecov.
|
|
another suggestion I have is to force any string input into lower case i.e. lower case SECONDARY_PREFERRED before running it through the case or validating in any way. In the mongo docs its all camel case. So kinda a bummer that we're inconsistent? Its out there now, in 2.5, but we should probably accept both the mongo standard as what we now have and deprecate the non mongo syntax? I know that we support more backing engines, but since we're only doing mongo for slave/secondary for now, why not be consistent?? |
…rse-server into futzing-with-read-preference
|
It was upper case because mongo node driver is upper case. But I agree it should handle both. So I turned it case insensitive. I also added support to choose read preference for includes and subqueries directly from API. |
There was a problem hiding this comment.
@davimacedo This all looks good to me -- and I can take a stab at the documentation, which I think I can now fill out much better, for you to review.
But I do have one observation which I'd like your feedback on. There are three options now:
- readPreference
- includeReadPreference
- subqueryReadPreference
I question that we need this. It's hard for me to imagine why we need this level of granularity and it adds complexity that I just don't think is worth its value. I'd think that if we simply had the option for readPreference and then set all include and subqueries to use the same readPreference that would be adequate. If there is a reason to have the granularity, I'd suggest that we should default to having readPreference set the other two, but give the ability to explicitly override in the case that it is needed. I know that this is not a trivial change. Let me know what you think.
@TylerBrock I'd be very interested in your feedback on this issue too.
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
…rence to follow readPreference by default
|
I've just rebased this request and also changed the subqueryReadPreference and includeReadPreference options behavior according to @acinader suggestion a while ago. Basically they now follow readPreference option by default, but can be override if wanted. |
|
ha. nice. |
* allow setting readpreference when using rest api. * take out partially complete unit test. * oops. nit * Include read preference option for find directly from api and adding few more tests * Adding catch for all tests * Keep same check for get and find * Turn read preference case insensitive * Includes and subqueries read preferences through API * Fixing bugs regarding changes that were done in master branch during the last year * Changing behavior to make includeReadPreference and subqueryReadPreference to follow readPreference by default
Contributing to #3959