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

Add support for auth db #20

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

nightowlengineer
Copy link

Fixes #19

I haven't yet had the opportunity to test this with the integration suite, but plan to do so shortly. If you're able to so in the meantime then feel free!

@ctrimble
Copy link
Contributor

I think we will need to inject some new tests, now that there are two ways to do authentication.

@nightowlengineer
Copy link
Author

Sure 👍 Once I get the integration tests working on the above, I'll try and add a couple of test cases in.

@nightowlengineer
Copy link
Author

After a lot of faffing around I can't get the tests working properly here. I'm on Windows and using Hyper-V, I was able to get Vagrant working with a slightly newer image and updated the provisioning script accordingly, but unfortunately the whole networking integration with the host is 'poor' at best - bridging my adapter to get access to the test instance completely blows the network throughput for the whole machine (50-60Mbps to 150Kbps...) which in turn makes the provisioning fail due to timeouts etc.

So unfortunately, I don't think I'm going to be able to work on the test side of this unless I can get around to moving this onto a Linux VM at some point, and realistically that is going to be at least a couple of weeks before I can get to it.

What would you like to do with this PR in the meantime? You could merge this onto a new branch on your repo, add the necessary tests, then pull it onto mainline that way?

@ctrimble
Copy link
Contributor

I am sorry to hear that the tests are not working on your machine. Vagrant is really just there to provide a way to launch a known version of Mongo to test against. Perhaps that needs to change.

Here are some options:

  • Vagrant is only needed for the integration tests. Use the skip property on the exec plugin to allow for testing against a local Mongo. Then just provide your own Mongo for testing.
  • As you suggested, merge the branch into a new branch in this repo. I would then add the tests and work it into the main line of development. This might cause the change to stall.

@nightowlengineer
Copy link
Author

Sounds good to me. I've just pushed some changes up for the tests to get them working in the first instance (and a bump in one of the plugins to work properly on Windows). The integration tests passed in full with these changes (bar skipping the Vagrant setup).

I'll try and get some additional tests in soon for the actual auth stuff.

@mharbeck
Copy link

Will this PR, by any chance, get merged soon?

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.

specifiy authentication database different to database
3 participants