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

feat: improve mongo support #25

Merged
merged 3 commits into from
Feb 17, 2022
Merged

Conversation

mhdawson
Copy link
Contributor

  • build url needed to connect
  • move auth info to the right nesting
  • add test for case when port is used

Signed-off-by: Michael Dawson mdawson@devrus.com

- build url needed to connect
- move auth info to the right nesting
- add test for case when port is used

Signed-off-by: Michael Dawson <mdawson@devrus.com>
@mhdawson
Copy link
Contributor Author

mhdawson commented Feb 17, 2022

@tchughesiv could you test this out. It should be used as:

let bindings;
try {
    // check if the deployment has been bound to a pg instance through
    // service bindings. If so use that connect info
    bindings = serviceBindings.getBinding('MONGODB', 'mongodb');
} catch (err) { // proper error handling here
};

const url = bindings.url + '/' + database + '?retryWrites=true&w=majority';
const client = new MongoClient(url, bindings.connectionOptions);

passing the connectionOptions to new MongoClient() will allow additional connections to be set through service bindings if/when they are supported/provided.

Note that I did move the username/password to the structure I found in options under new MongoClient in - https://mongodb.github.io/node-mongodb-native/3.4/api/MongoClient.html

This is technically breaking but since we are at a 0. version and nobody should be depending on this yet that is the best thing to do.

Once we confirm this works, we'll also have to fix up the Typscript .d.ts file to reflect that what is returned is not just a connectionOptions object.

- fix up the formatting based on testing with mongo client

Signed-off-by: Michael Dawson <mdawson@devrus.com>
Signed-off-by: Michael Dawson <mdawson@devrus.com>
@tchughesiv
Copy link
Contributor

tchughesiv commented Feb 17, 2022

@mhdawson database is not a requirement to connect to cluster

const url = bindings.url + '?retryWrites=true&w=majority';

or just

const client = new MongoClient(bindings.url, bindings.connectionOptions);

@mhdawson
Copy link
Contributor Author

@tchughesiv in terms of the database great if we don't need to add, ideally we could just pass in the results of what we get back from the bindings after having set any additional options needed on the bindings.connectionOptions object.

@mhdawson
Copy link
Contributor Author

@tchughesiv confirmed this PR works for him, landing.

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.

2 participants