Skip to content
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

Support for overriding MongoClientOptions in casbah driver #132

Merged
merged 2 commits into from
Nov 4, 2016

Conversation

dpfeiffer
Copy link
Contributor

Hi,

this PR solves #131. Actually doesn't allow to override settings specified in the URI because in the Java MongoDB Driver settings from the URI override those within the MongoClientOptions.

I decided to exclude some of the values from MongoClientOptions because they could be dangerous as discussed in #23 (readPreference, readConcern).

@natewarr
Copy link

natewarr commented Nov 2, 2016

👍 Nice timing. Was looking for this exact functionality.

Copy link
Owner

@scullxbones scullxbones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First - thanks for the contribution! This looks pretty good to me. Thanks for the docs and the spec.

Two things:

  • the options that refer to time (ms) - can they be changed from ints to typesafe config durations? It would make them easier to use.
  • the spec could use a test that verifies that given an overridden config, the values change. Just a couple fields is fine, don't need to change all of them.

Question - are the values used in the reference.conf the defaults for the builder?

@dpfeiffer
Copy link
Contributor Author

Hi,

  • sure, the naming is copied from (or follows) the naming convention of the URI config parameters. I propose to just remove just the ms but keep the names near to the URI parameter names.
  • will add that one
  • the values in the reference config are the ones from the builder, I first thought of using the builder as fallback, but for the RxClientSettings the values are also in the reference.conf so I decided to do it the same way

Copy link
Owner

@scullxbones scullxbones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@scullxbones scullxbones merged commit 79c3b8f into scullxbones:master Nov 4, 2016
JeanFrancoisGuena pushed a commit to JeanFrancoisGuena/akka-persistence-mongo that referenced this pull request Nov 4, 2016
…ones#132)

* specify mongodb client settings in reference.conf

* configure time values as FiniteDuration
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants