bigquery: api improvements #2530

callmehiphop · 2017-08-14T15:37:30Z

TODOs

Documentation
Unit Tests
System Tests

callmehiphop · 2017-08-14T17:23:23Z

@tswast if you have some free time, could you look through this PR to make sure all the P0 items look ok?

I also want to mention for CRUD calls on specific properties (labels, timePartitioning, etc.), this all happens through a setMetadata method, which is just a PATCH request that the user can supply an object to.

e.g.

var metadata = {
  labels: {
    foo: 'bar'
  }
};

table.setMetadata(metadata, (err, apiResponse) => {
  // ...
});

packages/bigquery/src/index.js

+        value = parseFloat(value);
+        break;
+      }
+      case 'INTEGER': {


packages/bigquery/src/index.js

+ *     // Error handling omitted.
+ *   }
+ *
+ *   job.getQueryResults(function(err, rows) {});


packages/bigquery/src/index.js

-    if (options.parameterMode === 'named') {
-      options.queryParameters = [];
+  if (query.params) {
+    reqOpts.useLegacySql = false;


packages/bigquery/src/job.js

+    }
+
+    var nextQuery = null;
+    if (resp.jobComplete === false) {


packages/bigquery/src/index.js

+ *     // Error handling omitted.
+ *   }
+ *
+ *   job.getQueryResults(function(err, rows) {});


packages/bigquery/src/index.js

+ * bigquery.createJob().then(function(data) {
+ *   var job = data[0];
+ *
+ *   return job.getQueryResults();


packages/bigquery/src/job.js

+    }
+
+    var nextQuery = null;
+    if (resp.jobComplete === false) {


packages/bigquery/src/index.js


-    callback(null, job, resp);
-  });
+  this.createJob(reqOpts, callback);


callmehiphop · 2017-09-25T13:26:19Z

@stephenplusplus @tswast I think I'm ready for a final review if you can find some time soon.

coveralls · 2017-09-25T13:32:52Z

Coverage remained the same at 100.0% when pulling 79bcb4d on callmehiphop:dg--bigquery-redesign into 118b622 on GoogleCloudPlatform:master.

packages/bigquery/src/dataset.js

+    request: function(reqOpts) {
+      if (reqOpts.method === 'PATCH' && reqOpts.json.etag) {
+        reqOpts.headers = reqOpts.headers || {};
+        reqOpts.headers['If-Match'] = reqOpts.json.etag;


stephenplusplus · 2017-09-25T15:24:53Z

Facing a system test error:

  1) BigQuery BigQuery/Table should insert rows via stream:
     Value '{{projectId}}' in content does not agree with value 'nth-circlet-705'. This can happen when a value set through a parameter is inconsistent with a value set in the request.
  ApiError
      at Object.parseHttpRespBody (node_modules\@google-cloud\common\src\util.js:192:30)
      at Object.handleResp (node_modules\@google-cloud\common\src\util.js:132:18)
      at Request._callback (node_modules\@google-cloud\common\src\util.js:255:14)
      at Request.self.callback (node_modules\request\request.js:186:22)
      at Request.<anonymous> (node_modules\request\request.js:1163:10)
      at IncomingMessage.<anonymous> (node_modules\request\request.js:1085:12)
      at endReadableNT (_stream_readable.js:1059:12)
      at _combinedTickCallback (internal/process/next_tick.js:138:11)
      at process._tickCallback (internal/process/next_tick.js:180:9)

This is a console.log of bigquery:

BigQuery {
  makeAuthenticatedRequest:
   { [Function: makeAuthenticatedRequest]
     getCredentials: [Function: bound getCredentials],
     authClient:
      Auth {
        authClientPromise: [Object],
        authClient: [Object],
        config: [Object],
        environment: {},
        projectId: 'nth-circlet-705' } },
  authClient:
   Auth {
     authClientPromise: Promise { [Object] },
     authClient:
      JWT {
        transporter: DefaultTransporter {},
        clientId_: undefined,
        clientSecret_: undefined,
        redirectUri_: undefined,
        opts: {},
        credentials: [Object],
        email: '[email protected]',
        keyFile: undefined,
        key: '-----BEGIN PRIVATE KEY----- ... -----END PRIVATE KEY-----\n',
        scopes: [Array],
        subject: undefined,
        gToken: [Function: GoogleToken],
        projectId: 'nth-circlet-705',
        gtoken: [Object] },
     config:
      { baseUrl: 'https://www.googleapis.com/bigquery/v2',
        scopes: [Array],
        packageJson: [Object] },
     environment: {},
     projectId: 'nth-circlet-705' },
  baseUrl: 'https://www.googleapis.com/bigquery/v2',
  getCredentials: [Function: bound getCredentials],
  globalInterceptors: [],
  interceptors: [],
  packageJson: { ... },
  projectId: '{{projectId}}',
  projectIdRequired: true,
  Promise: [Function: Promise] }

packages/bigquery/src/dataset.js

@@ -249,33 +260,13 @@ Dataset.prototype.createTable = function(id, options, callback) {
    options = {};
  }

-  var body = extend(true, {}, options, {


packages/bigquery/test/dataset.js

+    var FAKE_QUERY = 'SELECT * FROM `table`';
+
+    it('should call through to bigQuery#startQuery', function(done) {
+      ds.bigQuery.startQuery = function() {


packages/bigquery/src/index.js

+
+    switch (schemaField.type) {
+      case 'BOOLEAN':
+      case 'BOOL': {


packages/bigquery/src/index.js

+        break;
+      }
+      case 'FLOAT':
+      case 'FLOAT64': {


packages/bigquery/src/index.js

+        break;
+      }
+      case 'INTEGER':
+      case 'INT64': {


packages/bigquery/src/table.js

@@ -1064,12 +871,21 @@ Table.prototype.import = function(source, metadata, callback) {
 *
 * @param {object|object[]} rows - The rows to insert into the table.
 * @param {object=} options - Configuration object.
+ * @param {boolean} options.autoCreate - Automatically create the table if it


packages/bigquery/src/table.js

-  if (metadata.name) {
-    metadata.friendlyName = metadata.name;
-    delete metadata.name;
+  if (is.fn(metadata)) {


packages/bigquery/src/table.js

+    }
+  });
+
+  if (is.fn(metadata)) {


packages/bigquery/test/table.js

+      table.startCopyFrom(SOURCE_TABLE, done);
+    });
+
+    it('should pass an error to the callback', function(done) {


packages/bigquery/test/table.js

+      table.bigQuery.createJob = function() {};
+    });
+
+    it('should send the correct API request', function(done) {


callmehiphop · 2017-09-26T01:01:16Z

@dwmclary for whatever reason it won't let me add you as a reviewer, so I hope a friendly ping here will suffice.

callmehiphop · 2017-09-28T19:25:36Z

@stephenplusplus in regard to #2530 (comment) - are you using a specific OS/Node version/etc.? I can't reproduce it on my machine.

stephenplusplus · 2017-09-28T19:34:29Z

Windows 10, Node v8.6.0, npm 5.3.0.

It might depend on how you are auth'ing. From table.createWriteStream():

      request: {
        uri: format('{base}/{projectId}/jobs', {
          base: 'https://www.googleapis.com/upload/bigquery/v2/projects',
          projectId: self.bigQuery.projectId
        })
      }

I'm using ADC, so self.bigQuery.projectId isn't set. This probably isn't new to this PR, but maybe sneak a fix in?

callmehiphop · 2017-09-28T20:15:45Z

@stephenplusplus ah, gotcha. I'm wondering what the most correct solution here is, initially I would think to call util.replaceProjectIdToken(...) on all the options, but that feels wrong. If you think it's more involved than that, I would ask we open a separate issue so we don't delay the release of these changes.

coveralls · 2017-09-28T20:30:16Z

Coverage remained the same at 100.0% when pulling 6631a1e on callmehiphop:dg--bigquery-redesign into 118b622 on GoogleCloudPlatform:master.

stephenplusplus · 2017-09-28T21:01:44Z

That makes sense, re: separate effort.

coveralls · 2017-10-05T22:23:05Z

Coverage remained the same at 100.0% when pulling d5364b0 on callmehiphop:dg--bigquery-redesign into 118b622 on GoogleCloudPlatform:master.

callmehiphop · 2017-10-05T22:55:09Z

@stephenplusplus I think we're still waiting for an approval from @dwmclary, but is there anything else I may have forgotten to tackle in this?

stephenplusplus · 2017-10-10T21:26:43Z

@callmehiphop want to just merge this?

callmehiphop added 8 commits August 11, 2017 15:28

bigquery: api redesign

e9949aa

generate job ids on client

4157154

add convenience option for timePartitioning

39b5e6a

add convenience option for views

4ced475

honor job prefix option for all job types

9ae7a87

add etag support for dataset/table patch requests

10c26aa

honor all table.create options within table.setMetadata

a41de5c

change table.setMetadata to use PATCH instead of PUT

fdeeab2

callmehiphop added api: bigquery Issues related to the BigQuery API. status: do not merge labels Aug 14, 2017

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 14, 2017

callmehiphop added 2 commits August 22, 2017 16:39

add support for dryRun option

009da2c

publicly document bigquery#createJob

af1fc34

tswast reviewed Aug 22, 2017

View reviewed changes

callmehiphop added 7 commits August 23, 2017 22:49

create table on 404 when inserting data

0842175

throw error when no rows are provided

febc380

create high & low level methods for all job related rpcs

66eb0f7

add support for setting job metadata

ccdae20

add support for setting table metadata

38e8e0e

set useLegacySql default value to false

3c5a971

add high-level table operation methods

c6ecfec

tswast reviewed Sep 7, 2017

View reviewed changes

tswast approved these changes Sep 12, 2017

View reviewed changes

callmehiphop added 2 commits September 12, 2017 16:06

implement/refactor create table on insert

0bb1b31

unit tests

2c9d6b3

callmehiphop requested a review from lukesneeringer as a code owner September 25, 2017 05:58

callmehiphop removed the status: do not merge label Sep 25, 2017

callmehiphop added 2 commits September 25, 2017 02:13

linting

381fb4f

fixed snippet lint error

79bcb4d

googleapis deleted a comment from coveralls Sep 25, 2017

stephenplusplus reviewed Sep 25, 2017

View reviewed changes

stephenplusplus reviewed Sep 26, 2017

View reviewed changes

minor adjustments per pr feedback

6631a1e

fixed snippet lint error

d5364b0

stephenplusplus approved these changes Oct 9, 2017

View reviewed changes

callmehiphop merged commit a143927 into googleapis:master Oct 16, 2017

renovate bot mentioned this pull request Feb 20, 2018

Update dependency @google-cloud/error-reporting to v0.2.3 - autoclosed nLight/objektiv#18

Closed

bigquery: api improvements #2530

bigquery: api improvements #2530

Conversation

callmehiphop commented Aug 14, 2017 • edited Loading

TODOs

callmehiphop commented Aug 14, 2017

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.

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.

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.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

callmehiphop commented Sep 25, 2017

coveralls commented Sep 25, 2017

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

stephenplusplus commented Sep 25, 2017

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.

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.

callmehiphop commented Sep 26, 2017

callmehiphop commented Sep 28, 2017

stephenplusplus commented Sep 28, 2017

callmehiphop commented Sep 28, 2017

coveralls commented Sep 28, 2017

stephenplusplus commented Sep 28, 2017

coveralls commented Oct 5, 2017

callmehiphop commented Oct 5, 2017

stephenplusplus commented Oct 10, 2017

callmehiphop commented Aug 14, 2017 •

edited

Loading