compute onComplete example should show "the right way" to attach handlers?

[jgeewax]

On https://googlecloudplatform.github.io/gcloud-node/#/docs/v0.24.1/compute/operation?method=onComplete

operation.onComplete(function(err, metadata) {
  if (err.code === 'OPERATION_INCOMPLETE') {
    // The operation is not complete yet. You may want to register another
    // `onComplete` listener or queue for later.
  }

  if (!err) {
    // Operation complete!
  }
});

This example doesn't really show me how I'm supposed to continue attaching handlers (and I would actually need to redesign the flow of my code to make this work).

Can we update the example to show exactly what "the right way" to do this is?

[stephenplusplus]

If we actually want to support "wait until it's done no matter what", we can maybe just say "pass maxAttempts: 0", which we can change our code to treat as "infinite"?

[jgeewax]

I think both are good things to do...

It seems kind of weird that we have a handler called onComplete that can be called with an error of "OPERATION_INCOMPLETE"... Shouldn't we just not call the handler then ?

[stephenplusplus]

onComplete that can be called with an error of "OPERATION_INCOMPLETE"

The error should really be considered as "OPERATION_DID_NOT_COMPLETE_WITHIN_ALLOWED_TIME".

The concept of how the method works, re: attempts/being called with an error, is explained in the method description:

If the operation doesn't complete after the maximum number of attempts have been made (see options.maxAttempts and options.interval), an error will be provided to your callback with code: OPERATION_INCOMPLETE.

Shouldn't we just not call the handler then ?

That makes sense, but I think we'd hear from developers saying "I need to know when all attempts are exhausted"-- it's just a way to give control/progress updates back to the user.

Supporting "call only when it's complete" would be ideal, but since it can be error-prone to implement-- onComplete is just a convenience wrapper of setTimeout(getMetadata)-- we have to rely on the network and a proper API response to indicate it's complete. If those things don't happen exactly as we expect, the loop will go in indefinitely, without the user knowing that it's happening or being able to stop it or check on the status... which is why we have sane defaults and allow overrides (they could provide maxAttempts: 92834 if they really wanted)

I think the easiest thing to do is what you originally wanted (:smile:); I'll just send a PR showing how to call onComplete again if you get the "DID_NOT_COMPLETE" error.

// @callmehiphop if you have any thoughts on how this should work.

[jgeewax]

but I think we'd hear from developers saying "I need to know when all attempts are exhausted"

Shouldn't that be .onTimeout() ?

[stephenplusplus]

That's interesting... we could turn it into a true event emitter:

zone.createVM(function(err, vm, operation) {
  operation
    .on('complete', function(metadata) {})
    .on('error', function(err) {})
    .on('timeout', function() {
      if (something) {
        this.startPolling(); // start again
      }
    });

  operation.startPolling([options]);
});

[jgeewax]

+1 to that.

[callmehiphop]

I agree, a real event emitter would be much nicer imo