{"action":"created","issue":{"url":"https://api.github.com/repos/GoogleCloudPlatform/google-cloud-node/issues/1142","repository_url":"https://api.github.com/repos/GoogleCloudPlatform/google-cloud-node","labels_url":"https://api.github.com/repos/GoogleCloudPlatform/google-cloud-node/issues/1142/labels{/name}","comments_url":"https://api.github.com/repos/GoogleCloudPlatform/google-cloud-node/issues/1142/comments","events_url":"https://api.github.com/repos/GoogleCloudPlatform/google-cloud-node/issues/1142/events","html_url":"https://github.com/GoogleCloudPlatform/google-cloud-node/pull/1142","id":137323022,"number":1142,"title":"core: add support for promises","user":{"login":"callmehiphop","id":1707267,"avatar_url":"https://avatars.githubusercontent.com/u/1707267?v=3","gravatar_id":"","url":"https://api.github.com/users/callmehiphop","html_url":"https://github.com/callmehiphop","followers_url":"https://api.github.com/users/callmehiphop/followers","following_url":"https://api.github.com/users/callmehiphop/following{/other_user}","gists_url":"https://api.github.com/users/callmehiphop/gists{/gist_id}","starred_url":"https://api.github.com/users/callmehiphop/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/callmehiphop/subscriptions","organizations_url":"https://api.github.com/users/callmehiphop/orgs","repos_url":"https://api.github.com/users/callmehiphop/repos","events_url":"https://api.github.com/users/callmehiphop/events{/privacy}","received_events_url":"https://api.github.com/users/callmehiphop/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/GoogleCloudPlatform/google-cloud-node/labels/cla:%20yes","name":"cla: yes","color":"009800"},{"url":"https://api.github.com/repos/GoogleCloudPlatform/google-cloud-node/labels/core","name":"core","color":"f7c6c7"},{"url":"https://api.github.com/repos/GoogleCloudPlatform/google-cloud-node/labels/don't%20merge","name":"don't merge","color":"e11d21"},{"url":"https://api.github.com/repos/GoogleCloudPlatform/google-cloud-node/labels/minor","name":"minor","color":"eb6420"}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":26,"created_at":"2016-02-29T17:48:28Z","updated_at":"2016-09-21T21:51:02Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/GoogleCloudPlatform/google-cloud-node/pulls/1142","html_url":"https://github.com/GoogleCloudPlatform/google-cloud-node/pull/1142","diff_url":"https://github.com/GoogleCloudPlatform/google-cloud-node/pull/1142.diff","patch_url":"https://github.com/GoogleCloudPlatform/google-cloud-node/pull/1142.patch"},"body":"Fixes #551\r\n\r\nThis PR aims to add support for [Promises](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise).\r\n\r\nThis will also introduce a breaking change moving all stream related APIs behind a `stream` option.\r\n\r\nBecause this will be such an enormous change and GitHub probably won't be able to display everything, logical commits will be used to make reviewing the changes a little easier.\r\n\r\n#### TODO:\r\n\r\n- [ ] Core support\r\n- [ ] Service support - this includes unit tests, system tests and documentation.\r\n  - [ ] BigQuery\r\n  - [ ] Compute\r\n  - [ ] Datastore\r\n  - [ ] DNS\r\n  - [ ] Logging\r\n  - [ ] Prediction\r\n  - [ ] PubSub\r\n  - [ ] Resource\r\n  - [ ] Search\r\n  - [ ] Storage\r\n  - [ ] Translate\r\n- [ ] Documentation site support"},"comment":{"url":"https://api.github.com/repos/GoogleCloudPlatform/google-cloud-node/issues/comments/248753621","html_url":"https://github.com/GoogleCloudPlatform/google-cloud-node/pull/1142#issuecomment-248753621","issue_url":"https://api.github.com/repos/GoogleCloudPlatform/google-cloud-node/issues/1142","id":248753621,"user":{"login":"jmdobry","id":1213510,"avatar_url":"https://avatars.githubusercontent.com/u/1213510?v=3","gravatar_id":"","url":"https://api.github.com/users/jmdobry","html_url":"https://github.com/jmdobry","followers_url":"https://api.github.com/users/jmdobry/followers","following_url":"https://api.github.com/users/jmdobry/following{/other_user}","gists_url":"https://api.github.com/users/jmdobry/gists{/gist_id}","starred_url":"https://api.github.com/users/jmdobry/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jmdobry/subscriptions","organizations_url":"https://api.github.com/users/jmdobry/orgs","repos_url":"https://api.github.com/users/jmdobry/repos","events_url":"https://api.github.com/users/jmdobry/events{/privacy}","received_events_url":"https://api.github.com/users/jmdobry/received_events","type":"User","site_admin":false},"created_at":"2016-09-21T21:51:02Z","updated_at":"2016-09-21T21:51:02Z","body":"Looking at the implementation of `gax-nodejs`, I'm thinking there would be a rather unfortunate loss of information/loss of functionality if the hand-written methods were to return a Promise, as opposed to returning the event emitter. For example:\r\n\r\n```js\r\nvar Language = require('@google-cloud/language');\r\n\r\nvar language = Language();\r\nvar api = Language.v1beta1();\r\n\r\nvar document = {\r\n  type: 'PLAIN_TEXT',\r\n  content: 'Hello, world!'\r\n};\r\n\r\n// The hand-written method returns just a promise\r\nvar promise = language.detectSentiment(document.content);\r\n\r\n// gax-nodejs returns an event emitter\r\nvar apiRequest = api.analyzeSentiment(document);\r\n\r\ntry {\r\n  // uh oh, how to I cancel the request\r\n  promise.cancel();\r\n} catch (err) {\r\n  // Uncaught TypeError: promise.cancel is not a function\r\n}\r\n\r\n// Nice, this works\r\napiRequest.cancel();\r\n\r\npromise\r\n  .then((sentiment) => {\r\n    // ...\r\n  })\r\n  .catch((err) => {\r\n    // ...\r\n  });\r\n\r\n// This works today\r\napiRequest.result\r\n  .then((response) => {\r\n    // ...\r\n  })\r\n  .catch((err) => {\r\n    // ...\r\n  });\r\n```\r\n\r\nI'm thinking the hand-written methods should just return the event emitter returned by `gax-nodejs`:\r\n\r\n```js\r\nvar languageRequest = language.detectSentiment(document.content);\r\nvar apiRequest = api.analyzeSentiment(document);\r\n\r\n// Yay, I can cancel the request!\r\nlanguageRequest.cancel();\r\n\r\n// Nice, this works\r\napiRequest.cancel();\r\n\r\n// Access the promise on the \"result\" property\r\nlanguageRequest.result\r\n  .then((sentiment) => {\r\n    // ...\r\n  })\r\n  .catch((err) => {\r\n    // ...\r\n  });\r\n\r\n// This works today\r\napiRequest.result\r\n  .then((response) => {\r\n    // ...\r\n  })\r\n  .catch((err) => {\r\n    // ...\r\n  });\r\n```\r\n\r\nUse the event emitter in combination with callback-style:\r\n\r\n```js\r\nvar languageRequest = language.detectSentiment(document.content, function (err, sentiment) {\r\n  // ...\r\n});\r\nvar apiRequest = api.analyzeSentiment(document, function (err, response) {\r\n  // ...\r\n});\r\n\r\n// Yay, I can cancel the request!\r\nlanguageRequest.cancel();\r\n\r\n// Nice, this works\r\napiRequest.cancel();\r\n```\r\n\r\nFor those methods that optionally return a stream, the method will have to be split into two methods:\r\n\r\n`Table#getRows` => `Table#getRows` and `Table#getRowsStream`:\r\n\r\n```js\r\n// callback-style\r\ntable.getRows(function (err, rows) {\r\n  // ...\r\n});\r\n\r\n// promise-style\r\ntable.getRows().result.then(function (rows) {\r\n  // ...\r\n}).catch(function (err) {\r\n  // ...\r\n});\r\n\r\n// stream\r\ntable.getRowsStream()\r\n  .on('error', console.error)\r\n  .on('data', function (row) {})\r\n  .on('end', function () {\r\n    // All rows have been retrieved.\r\n  });\r\n```"}}
{"actor":{"display_login":"jmdobry"}}