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

Batch interface suggestion #9

Open
fanatid opened this issue Mar 29, 2015 · 4 comments
Open

Batch interface suggestion #9

fanatid opened this issue Mar 29, 2015 · 4 comments

Comments

@fanatid
Copy link
Contributor

fanatid commented Mar 29, 2015

Now batch call look like:

rpc.batch(function() { /* add some request to rpc */ }, function() { /* request callback */ })

I think it's not good, why I must write yet one function for filling rpc.batchedCalls?

And what really important, what happened if we get an exception in first function? When we try make new request (getInfo for example), batchedCalls not be null and rpc instance will be considered that we fill batch request... so, it's broke all rpc instance!

I suggest replaced batch request by something like this:

rpc.createBatch() // create copy of rpc instance and set batchedCalls as empty array
  .getRawTransaction(txid1) // put request to batchedCalls
  .getRawTransaction(txid2) // put request to batchedCalls
  .call(function() { /* request callback */ }) // make request

I'm not sure about createBatch, call and hope somebody offer more suitable names.

@maraoz
Copy link
Contributor

maraoz commented Mar 30, 2015

I agree that the interface can be improved, and I like your suggestion. I'd personally prefer:

rpc.batch() 
  .getRawTransaction(txid1)
  .getRawTransaction(txid2)
  .call(function() { /* request callback */ })

or a more promise-oriented version:

rpc.batch() 
  .getRawTransaction(txid1)
  .getRawTransaction(txid2)
  .call()
  .then(function() {
    console.log('success!');
  })
  .catch(function(error) {
    console.log('Batch call errored:', error);
  });

do you want to try and implement this in a PR (either of the versions)?

@braydonf
Copy link
Contributor

The batch API for levelup I think makes a fair amount of sense:

@fanatid
Copy link
Contributor Author

fanatid commented Mar 30, 2015

Promise not good idea here, because except batch bitcoind-rpc have other many methods.. and we should stick one style: callbacks everywhere or promise everywhere.
Bluebird have cool function promisifyAll, so I think choice towards to callbacks is obvious.
Thanks @braydonf clear must be added to batch (as in levelup)

@fanatid
Copy link
Contributor Author

fanatid commented Aug 22, 2015

FYI, I wrote my own bitcoind rpc driver with batch behavior described here, you can see code here

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

No branches or pull requests

3 participants