add maxInProgress option

[stephenplusplus]

Continuing from #649

To Dos:

• Tests
• See what @callmehiphop has to say.

[googlebot]

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

[googlebot]

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

[stephenplusplus]

@leibale - as @googlebot says, please confirm you're okay with my incorporation of your work here :)

Also, please take a look when you can to make sure this works for you.

[leibale]

how confirm that? i already signed on CLA

[ryanseys]

There's no formal way of telling the bot we are ok with it, it's not a very smart bot. :) Just confirm with us that you're cool with these changes.

[jgeewax]

I just double checked -- leibale signed the CLA.

[leibale]

@stephenplusplus my code working like a charm 😉.
my worker (in production) working with my fork repository few days.

[stephenplusplus]

@leibale great :) I was hoping you could test this branch out as well, as I had to make some changes to your original work.

$ npm install --save stephenplusplus/gcloud-node#spp--pubsub-add-maxInProgress

[leibale]

what about that?

[stephenplusplus]

The code in this PR is different than what you originally sent us. Did you try out the code from this PR?

[leibale]

in "stephenplusplus/gcloud-node#spp--pubsub-add-maxInProgress" branch no option to mark message as done without ack, i cant test..

[stephenplusplus]

Can you explain that with some example code?

[leibale]

subscription.on('message', function (message) {
  request.get(message.data.url, function (err) {
    if (err) { // return the message to the queue
      return subscription.done(); // inProgress--
    }

    subscription.ack(message.ackId);
  });
});

[stephenplusplus]

Okay, I think I misunderstood the idea originally, then. I think what you want is basically to be able to pause and resume the pulling? Would just adding .pause() and .resume() methods work for your use case?

[leibale]

no.. if i try to process message and i got error i need to down the inProgress without ack the message.

my english so bad :(

[leibale]

and..?

[stephenplusplus]

Sorry @leibale, holiday weekend and #701 blew up my mind today 💣 👨 ☁️. I'll be revisiting this soon, and don't worry, the misunderstandings are my fault only.

[leibale]

i want to remove my fork and go back to master branch- time estimation for this enhancement?

[stephenplusplus]

Sorry for the delays. I hopefully addressed this now with a new method, skip. So, from your previous example, this would look like:

subscription.on('message', function (message) {
  request.get(message.data.url, function (err) {
    if (err) {
      return message.skip();
    }

    subscription.ack(message.ackId); 
  });
});

Both skip and ack will -- the counter. Please try it out!

[leibale]

now it's looks good- i try it tomorrow

[leibale]

ack doesn't -- the counter (only skip)

[stephenplusplus]

Woops, good catch. Thanks!

[leibale]

@stephenplusplus 👍
now i just want this on the master and.. :P

[stephenplusplus]

Haha, me too! As long as you give me longer than an hour :) I hope to get a lot of the outstanding PRs merged by the end of the week, and if that goes well, I'll speed this one up.

[leibale]

ok. thanks alot for everything. :)

[leibale]

another mistake- you need to do the -- only if !err

[googlebot]

CLAs look good, thanks!

[stephenplusplus]

Thanks! Fixed.

[googlebot]

CLAs look good, thanks!

[stephenplusplus]

@callmehiphop PTAL.

[callmehiphop]

LGTM 👍

[stephenplusplus]

Thanks for the review and thanks for the idea @leibale!