dmanjunath / node-redshift

A simple collection of tools to help you get started with Amazon Redshift from node.js
69 stars 48 forks source link

Added queryPromise method for users to utilize promises instead of callbacks #11

Closed dYale closed 7 years ago

dYale commented 7 years ago

I was using the library and added this prototype method so that I could create a Promise.all() array for multiple queries. Figured it would be useful for others too.

dmanjunath commented 7 years ago

@dYale thanks for the PR! So I like this idea of utilizing promises a lot, but it doesn't seem like the right approach to me to have a separate API for a promise style return. The reason is because this function is only limited to the query function itself, not to any of the other instance functions like the CRUD functions. So if we need a new promise function to wrap every callback function, that'll create a lot more API's

So what do you think about this approach: I can use bluebird to modify the API's to accept either promises or callbacks, so you can pass in either return type. Here's a proof of concept https://jsfiddle.net/4xqabsve/1/

dmanjunath commented 7 years ago

@dYale I created two commits to add support for promises. 2f4c4b33c3f366ef990cab1a9ce684b745118d5f and 072b11c80f0f8696bc5f8d17573e8a195c1c417e. I'll push up a update to the readme but the usage is:

redshiftConnection.query('SELECT * FROM "Table" LIMIT 10') .then(data => { console.log(data); });//error handlers, catch, chaining whatever else

You can also do Promise.all like this

var q1 = redshiftConnection.query('SELECT FROM "Table" LIMIT 10 OFFSET 0'); var q2 = redshiftConnection.query('SELECT FROM "Table" LIMIT 10 OFFSET 10'); var q3 = redshiftConnection.query('SELECT FROM "Table" LIMIT 10 OFFSET 20'); var q4 = redshiftConnection.query('SELECT FROM "Table" LIMIT 10 OFFSET 30'); var q5 = redshiftConnection.query('SELECT * FROM "Table" LIMIT 10 OFFSET 40');

Promise.all([q1, q2, q3, q4, q5]).then(data => { console.log(data); }, err => { console.error(err) });

And the current callback API is also supported. If you wouldn't mind having a look and letting me know if this approach works for you that'd be awesome.

dYale commented 7 years ago

@dmanjunath This definitely works. I thought about pulling in Bluebird, but I didn't want to change too much of your existing library. This is definitely preferred over my PR.

Thanks for doing it!

dYale commented 7 years ago

No longer needed