Skip to content

Update node txn sample to pass results back from the transaction #1615

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

Merged

Conversation

taywrobel
Copy link
Contributor

Adjust the node.js transaction sample code to perform a SELECT as
part of the transaction and return the results through the txn
wrapper.

Addresses node component of issue #1606

Adjust the node.js transaction sample code to perform a SELECT as
part of the transaction and return the results through the txn
wrapper.

Addresses node component of issue cockroachdb#1606
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@cockroach-teamcity
Copy link
Member

@taywrobel
Copy link
Contributor Author

@jseldess @justinj

Following up on the changes made in #1603, this returns the updated balances within the transaction in the node sample code, addressing the node.js component of #1606

@@ -84,15 +84,20 @@ function transferFunds(client, from, to, amount, next) {
var acctBal = results.rows[0].balance;
if (acctBal >= amount) {
// Perform the transfer.
async.series([
async.waterfall([
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switched from series to waterfall here, since with waterfall only the last async step has it's results sent to the final async callback. Series returns the results of each function as separate callback parameters.

With series the outer function would need to have a signature along the lines of function (err, firstUpdateResults, secondUpdateResults, selectResults) { which I didn't think was as clear as this, where the transfer funds function returns just the updated funds.

Copy link
Contributor

@jseldess jseldess left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks very much, @twrobel3!

@jseldess
Copy link
Contributor

Also, @justinj tested your change, and everything works great. Merging.

@jseldess jseldess merged commit b231349 into cockroachdb:master Jun 22, 2017
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