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

Feature Request: Auto-Connect #109

Open
CatGuardian opened this issue Jan 1, 2018 · 2 comments
Open

Feature Request: Auto-Connect #109

CatGuardian opened this issue Jan 1, 2018 · 2 comments
Labels
enhancement This will add functionality to the Public API of the library and will result in a minor release investigating This issue requires more information to make a decision about how to tackle it

Comments

@CatGuardian
Copy link

I thought of a useful idea which came about from a problem I am struggling with. The problem is I don't think I should have to do 'await myCore.connect()' everywhere I want to do something that requires the Core to be already connected. For example: I shouldn't have to ' await db.connect()' before doing this:

// db is an instance of something that extends Core and db.Users is an instance of Model
const user = new User(db.Users, {
  name: 'First User',
});

const savedUser = await user.save();

The save() method should behind the scenes do an 'await db.connect()' before proceeding on with whatever save() normally does.

This should apply to all methods needing the database to be connected. Examples: find, get, select, etc...

This type of 'lazy connection' grabbing is done by another popular database library pg-promise. http://vitaly-t.github.io/pg-promise/index.html See Database section.

@notheotherben notheotherben added enhancement This will add functionality to the Public API of the library and will result in a minor release investigating This issue requires more information to make a decision about how to tackle it labels Jan 13, 2018
@notheotherben
Copy link
Member

Hi @CatGuardian,

This was something I did consider when creating Iridium, however I opted for the explicit approach for a number of reasons. Most notably, people tend to be bad at handling a wide range of error cases in their code (often just assuming the most obvious failure mode is the only one to deal with). In practice, an error thrown by the .save() method usually indicates an issue with the data being saved or a transient upstream issue. On the other hand, a failure to .connect() probably indicates that your application isn't configured correctly (if it's the first time it's being called) or your environment is not set up to spec.

By having an explicit .connect() method, it encourages people to handle errors relating to connection failures in a sensible manner, without polluting the various other interactions with error handling for connection issues.

Touching on the second (implicit) part of your question, the "best practice" for using Iridium's core is not to instantiate (or maintain an instance of it) for each request, but rather to have a singleton core for your application's lifecycle, which is instantiated and configured when the application starts and lasts until the application is shutdown.

This approach allows you to manage tasks such as schema migrations, index creation and data bootstrapping in a single, centralized location and ties well into the notion of an application "readiness" healthcheck (which only succeeds once the application has started and is ready to serve requests correctly).

It also takes advantage of the connection pooling support provided by the MongoDB client driver, as well as its robust support for re-connection and live topology changes.

import {MyCore, bootstrap} from "./db"
import {Application} from "./Application"

const config = require("./config.json")

const core = new MyCore(config.db)
console.log("Connecting to DB")
await core.connect()

console.log("Bootstrapping DB")
await bootstrap(core)

const app = new Application(core, config.app)

console.log(`Listening on ${config.app.port}`)
await app.listen()

Let me know if you still feel there's a strong motivation for including auto-connect in spite of this, or if the approach above solves your requirements.

Regards,

@CatGuardian
Copy link
Author

CatGuardian commented Jan 20, 2018

Thank you for your detailed answer.
And yes a solution where I could import it once and await on the connect once and then export the singleton would be fantastic! And that is what I tried doing. However typescript gave me an error saying I cannot await on something that isn't in an "async" method.

So your example doesn't really work. I use express if that helps you to help me. But if u can think of a way to export a singleton that is already connected to the database then that would help me tremendously.

Also I am trying to write Mocha unit tests so I was hoping for a clean way where I didn't need to set up the database every time in each individual mocha file. That could be accomplished by having the save() method await connect() for me and then I could just await save() in the unit test and wouldn't have to worry about 'await connect()' first. But this isn't as big of a deal I suppose I could await connect in every file.

I'm really more concerned with how to make a singleton and exporting it so I can import the already awaited and connected db.

Oh it might help to understand that I have my database layer in its own npm package and I am importing my database npm package into my API node.js app. So ideally I'm looking for something like "import my various Instance or Model classes" instead of importing the entire 'MyDatabase extends Core' kind of thing because I feel like we shouldn't import more than necessary in files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This will add functionality to the Public API of the library and will result in a minor release investigating This issue requires more information to make a decision about how to tackle it
Projects
None yet
Development

No branches or pull requests

2 participants