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

Make it possible to run with auth, as a replica set, and with additional args. #79

Merged
merged 4 commits into from
Sep 27, 2018

Conversation

jloveridge
Copy link
Contributor

feat(MongoInstance): add new option instance.args to allow passing additional args.
feat(MongoInstance): add new option instance.auth to allow running with auth enabled.
feat(MongoInstance): add new option instance.replSet to specify replica set name.

@nodkz
Copy link
Collaborator

nodkz commented Sep 27, 2018

Related comment #74 (comment)

@jloveridge Thanks for PR I ready to merge it!

But if you able to write a new class for ReplicaSet - MongoReplicaSet.js and put it near MongoMemoryServer.js with combined code from your comment above and existed code from MongoMemoryServer it will be amazing.

Let me know to merge PR or wait for MongoReplicaSet class from you.

@jloveridge
Copy link
Contributor Author

@nodkz I don't necessarily have an issue with creating another class. I do question whether or not it is necessary. The functionality I added is specific to passing command line arguments to the spawned mongod process. I don't really feel that justifies a separate class as we are still just spawning a single mongod instance. Now, if you are wanting to add some tooling around spinning up a replica set instead of just a single instance that is where I feel an additional class might be applicable. I will outline a brief overview of what something like that might look like.

  • New class MongodbMemoryReplSet.
    • This server type differs from the standard MongodbMemoryServer in the following ways:
      • Internally it will use 1 or more instances of MongodbMemoryServer.
      • Starting the server will spin up the instances of MongodbMemoryServer and initialize
        the replica set. This would be done by performing the initialization on the first instance created
        and then adding each of the other instances as members. This will necessarily rely on the ability
        to pass some of the very configuration options that I just made possible in these changes for
        MongodbMemoryServer and MongodbInstance .
      • Starting, stopping, and initialization of the replica set members would all be handled internally.
      • Starting the server would not resolve until all members have been spun up and the replica set
        is in a healthy state.
      • Add convenience methods to allow step-down and simulated failure of replica set members.
      • getConnectionString would return a string that includes all of the replica set members and
        look something like: mongodb://127.0.0.1:27333,127.0.0.1:27334,127.0.0.1:27335/dbName.
  • New options type: MongoMemoryReplSetOptsT.
    • Differences from MongoMemoryServerOptsT
      • Instead of an instance parameter there would be instances which would be an
        array of options for each instance that will be spawned.
      • Add a replSet property that allows specifying replica set specific configuration items such
        as the name of the replica set (--replSet), oplog size (--oplogSize), and possibly the number
        of member nodes to spin up (this one has no analog to mongod command line params).

So, while I do believe the above additions would be nice I feel they are outside the scope of just being able to pass additional command line arguments when starting an instance of mongod which is effectively what this PR addresses. I would suggest we merge this PR as is and a separate issue be created to make spinning up and management of a replica set less onerous on people who want/need them. As I commented on issue #74 it is fairly easy for someone to utilize what I have already done and get up and running with a single node replica set right now.

@jloveridge
Copy link
Contributor Author

I should have mentioned that I would be happy to tackle the creation of the new MongoMemoryReplicaSet implementation that I mentioned above. I just feel it should be addressed separately from this PR.

@nodkz
Copy link
Collaborator

nodkz commented Sep 27, 2018

@jloveridge Agreed! Let move new class to another PR.
Please add info about new options to README.md and I'll publish a new version.

Tnx.

@nodkz
Copy link
Collaborator

nodkz commented Sep 27, 2018

And typescript definitions ;)

@nodkz
Copy link
Collaborator

nodkz commented Sep 27, 2018

I'm already past midnight. But about 20 minutes will be online. And eeady to merge changes from mobile.

PS. Your overview is brilliant!

@jloveridge
Copy link
Contributor Author

Yep, going to add type definitions now since you have merged my other PR. Just need a few minutes.

Jarom Loveridge added 4 commits September 27, 2018 13:42
…additional args.

feat(MongoInstance): add new option `instance.auth` to allow running with auth enabled.
feat(MongoInstance): add new option `instance.replSet` to specify replica set name.
@jloveridge
Copy link
Contributor Author

Types have been added. Build passed for me locally. I expect Travis CI will pass very soon.

@nodkz nodkz merged commit 21a0769 into typegoose:master Sep 27, 2018
@nodkz
Copy link
Collaborator

nodkz commented Sep 27, 2018

It merged with master and after CI run it will publish a new version.

Thanks a lot for great PRs!! 👍

Now I can go sleep 😉

@nodkz
Copy link
Collaborator

nodkz commented Sep 27, 2018

🎉 This PR is included in version 2.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@jloveridge jloveridge deleted the feature/support-replset branch September 27, 2018 20:02
@jloveridge
Copy link
Contributor Author

Thank you very much. Now I can use the official package instead of a fork. Sleep well.

@StefanoDeVuono
Copy link
Contributor

I noticed this doesn't actually pass the '--auth' option to the command line. Is this intentional? I use it sometimes and I'm working on a PR to do just that, if anyone else is interested.

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