Skip to content

in repliace env, set hosts as Array to new Mongolian #102

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

muddydixon
Copy link

We have replica server info in Array

var hosts = ["localhost:27017", "localhost:27018", "localhost:27019"];

In master branch, we must set each arguments to create mongolian client to replica set, such that

var server = new Mongolian("localhost:27017", "localhost:27018", "localhost:27019");

But the number of servers are not specified in code, but are specified in config.
So in this patch, we can create mongolian client such that,

var hosts = ["localhost:27017", "localhost:27018", "localhost:27019"];
var server = new Mongolian(hosts);

@travisbot
Copy link

This pull request fails (merged 72de02d into f1a6bdc).

@travisbot
Copy link

This pull request passes (merged 72de02d into f1a6bdc).

@travisbot
Copy link

This pull request passes (merged 2ff62ea into f1a6bdc).

@marcello3d
Copy link
Owner

This solution feels a bit hacky. I think it can be simplified. What if we only handled two cases: arguments and array? Then you could have something like:

var args = arguments.length == 1 && arguments[0] instanceof Array ? arguments[0] : arguments

And the remainder of the code would be nearly identical.

Even with that, I'm not convinced this is the right solution, though. Maybe options.host should support an array instead? E.g. new Mongolian({ host:array }).

In either case, I would like to see the Mongolian constructor get simpler, not more complex.

@muddydixon
Copy link
Author

Thank you for you reply.
I consider more...

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