LRO implementation. #64

landrito · 2016-11-03T01:36:12Z

No description provided.

codecov-io · 2016-11-03T17:03:34Z

Current coverage is 92.94% (diff: 93.65%)

Merging #64 into master will increase coverage by 0.08%

@@             master        #64   diff @@
==========================================
  Files             9         10     +1   
  Lines          1037       1163   +126   
  Methods         157        180    +23   
  Messages          0          0          
  Branches        215        233    +18   
==========================================
+ Hits            963       1081   +118   
- Misses           74         82     +8   
  Partials          0          0

Powered by Codecov. Last update 49fb4f5...a5fc41b

jmuk

Overall, nice work! Looks like it's in a good shape.
A few nitpicking comments.

lib/lro.js

+    return;
+  }
+
+  var operationsApi = settings.longrunning.operationsApi;


lib/lro.js

+        canceller.callback(error);
+        return;
+      }
+      canceller.callback(null, op.result.response);


lib/lro.js

+        error = new Error(op.result.error.message);
+        error.code = op.result.error.code;
+        canceller.callback(error);
+        return;


lib/lro.js

+      } else {
+        var response = op.result.response;
+        if (this.decoder) {
+          response = this.decoder(response);


landrito · 2016-11-21T17:09:15Z

@jmuk This is ready for another look.

lib/gax.js

+ * @property {anyDecoder=} longrunning.metadataDecoder - The decoder to unpack
+ *   the metadata message. If not provided, the metadata will be unpacked if
+ *   the proto descriptor of the any value is found in the longrunningDescriptor
+ *   protoDescriptorPool.


lib/gax.js

+ *   the proto descriptor of the any value is found in the longrunningDescriptor
+ *   protoDescriptorPool.
+ * @property {backoffSettings=} longrunning.backoffSettings - The backoff
+ *   settings used to poll operation.


lib/gax.js

@@ -58,6 +58,10 @@
 *   pageToken.
 * @property {boolean=} isBundling - If set to false and the call is configured
 *   for bundling, bundling is not performed.
+ * @property {boolean || BackoffSettings=} longrunning - If set to false and the


landrito · 2016-11-30T23:52:36Z

This is ready for another look @jmuk.

jmuk

Please modify index.js to expose the new LongrunningDescriptor.

lib/longrunning.js

+ * operations service client and unpacking mechanisms for the operation.
+ * @param {BackoffSettings} backoffSettings - The backoff settings used in
+ * in polling the operation.
+ * @private


jmuk

LGTM -- please wait for the toolkit change though.

landrito · 2016-12-05T19:34:54Z

Thanks for the review!

landrito assigned jmuk Nov 3, 2016

landrito force-pushed the lro branch from c03995e to c363b9e Compare November 3, 2016 17:03

landrito force-pushed the lro branch 3 times, most recently from c490159 to 87cd6a6 Compare November 3, 2016 18:13

landrito unassigned jmuk Nov 3, 2016

landrito force-pushed the lro branch 2 times, most recently from 02c70e0 to 037b973 Compare November 3, 2016 23:35

landrito assigned jmuk Nov 3, 2016

jmuk reviewed Nov 4, 2016

View reviewed changes

landrito mentioned this pull request Nov 7, 2016

Add LRO wrapper. googleapis/gax-ruby#47

Merged

2 tasks

jmuk reviewed Nov 8, 2016

View reviewed changes

lib/lro.js

} else {

var response = op.result.response;

if (this.decoder) {

response = this.decoder(response);

This comment was marked as spam.

Sign in to view

landrito added 3 commits November 18, 2016 09:46

Initial LRO commit

d7fbd99

Nit fixes

975223a

Added functionality canceling an operation.

9f13575

landrito force-pushed the lro branch 4 times, most recently from 308375b to ee372de Compare November 21, 2016 16:53

Tests

a01b15d

landrito force-pushed the lro branch from ee372de to a01b15d Compare November 21, 2016 16:54

landrito force-pushed the lro branch from 5275c4c to 3c88701 Compare November 21, 2016 17:29

Fixed issue in Streaming test

b6b1fec

landrito force-pushed the lro branch from 3c88701 to b6b1fec Compare November 21, 2016 17:29

jmdobry mentioned this pull request Nov 21, 2016

google-cloud-node idioms, for consideration #71

Closed

landrito changed the title ~~WIP: LRO implementation.~~ LRO implementation. Nov 21, 2016

jmuk reviewed Nov 21, 2016

View reviewed changes

Settings Fix.

ba8d4a7

landrito force-pushed the lro branch from f045238 to ba8d4a7 Compare November 21, 2016 21:16

Fixed decoders

a0f4a49

jmuk reviewed Nov 21, 2016

View reviewed changes

lib/gax.js

@@ -58,6 +58,10 @@

* pageToken.

* @property {boolean=} isBundling - If set to false and the call is configured

* for bundling, bundling is not performed.

* @property {boolean || BackoffSettings=} longrunning - If set to false and the

This comment was marked as spam.

Sign in to view

landrito added 3 commits November 21, 2016 15:23

doc fix

d405d75

Refactor to GCN style. Tests still needed.

ca13a01

Added tests.

330091c

landrito force-pushed the lro branch from cdd1111 to 330091c Compare November 25, 2016 22:48

landrito added 2 commits November 30, 2016 13:23

Use nexttick

c58528f

Merge branch 'master' of github.com:googleapis/gax-nodejs into lro

f678274

landrito force-pushed the lro branch 2 times, most recently from b924b40 to 8b56102 Compare November 30, 2016 23:47

Documentation and tests

241c5d1

landrito force-pushed the lro branch from 8b56102 to 241c5d1 Compare November 30, 2016 23:50

jmuk reviewed Dec 2, 2016

View reviewed changes

Exposed Longrunning Descriptor / Docfix

a5fc41b

landrito force-pushed the lro branch from bad24fd to a5fc41b Compare December 2, 2016 18:34

jmuk approved these changes Dec 3, 2016

View reviewed changes

landrito merged commit 566e5c9 into googleapis:master Dec 5, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LRO implementation. #64

LRO implementation. #64

landrito commented Nov 3, 2016

codecov-io commented Nov 3, 2016 •

edited

Loading

jmuk left a comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

landrito commented Nov 21, 2016

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

landrito commented Nov 30, 2016

jmuk left a comment

This comment was marked as spam.

This comment was marked as spam.

jmuk left a comment

landrito commented Dec 5, 2016

LRO implementation. #64

LRO implementation. #64

Conversation

landrito commented Nov 3, 2016

codecov-io commented Nov 3, 2016 • edited Loading

Current coverage is 92.94% (diff: 93.65%)

jmuk left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

landrito commented Nov 21, 2016

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

landrito commented Nov 30, 2016

jmuk left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

This comment was marked as spam.

jmuk left a comment

Choose a reason for hiding this comment

landrito commented Dec 5, 2016

codecov-io commented Nov 3, 2016 •

edited

Loading