Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add browsing data sample #201

Merged
merged 5 commits into from
Sep 4, 2016
Merged

Add browsing data sample #201

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bfe3791
First draft of browseTable sample
Sep 2, 2016
720ea9b
A few minor tweaks
Sep 2, 2016
b0aa116
Fix bad rebase
Sep 2, 2016
ec5e9ae
Address comments
Sep 2, 2016
83ebd51
Address comments
Sep 2, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 24 additions & 45 deletions bigquery/system-test/tables.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,6 @@ var options = {
schema: 'Name:string, Age:integer, Weight:float, IsMagic:boolean',
rows: rows
};
var srcDataset = options.dataset;
var srcTable = options.table;
var destDataset = generateUuid();
var destTable = generateUuid();

describe('bigquery:tables', function () {
before(function (done) {
Expand All @@ -49,34 +45,27 @@ describe('bigquery:tables', function () {
// Upload data.csv
bucket.upload(options.localFilePath, function (err) {
assert.ifError(err, 'file upload succeeded');
// Create srcDataset
bigquery.createDataset(srcDataset, function (err) {
assert.ifError(err, 'srcDataset creation succeeded');
// Create destDataset
bigquery.createDataset(destDataset, function (err) {
assert.ifError(err, 'destDataset creation succeeded');
done();
});
// Create dataset
bigquery.createDataset(options.dataset, function (err, dataset) {
assert.ifError(err, 'dataset creation succeeded');
done();
});
});
});
});

after(function (done) {
// Delete srcDataset
bigquery.dataset(srcDataset).delete({ force: true }, function () {
// Delete destDataset
bigquery.dataset(destDataset).delete({ force: true }, function () {
// Delete files
storage.bucket(options.bucket).deleteFiles({ force: true }, function (err) {
if (err) {
return done(err);
}
// Delete bucket
setTimeout(function () {
storage.bucket(options.bucket).delete(done);
}, 2000);
});
// Delete testing dataset/table
bigquery.dataset(options.dataset).delete({ force: true }, function () {
// Delete files
storage.bucket(options.bucket).deleteFiles({ force: true }, function (err) {
if (err) {
return done(err);
}
// Delete bucket
setTimeout(function () {
storage.bucket(options.bucket).delete(done);
}, 2000);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why these changes? Don't we still need to delete both srcDataset and destDataset?

});
});
});
Expand Down Expand Up @@ -167,26 +156,16 @@ describe('bigquery:tables', function () {
});
});

describe('copyTable', function () {
it('should copy a table between datasets', function (done) {
program.copyTable(srcDataset, srcTable, destDataset, destTable, function (err, metadata) {
describe('listRows', function () {
it('should list rows in a table', function (done) {
program.listRows(options.dataset, options.table, function (err, rows) {
assert.equal(err, null);
assert.deepEqual(metadata.status, { state: 'DONE' });

bigquery.dataset(srcDataset).table(srcTable).exists(
function (err, exists) {
assert.equal(err, null);
assert.equal(exists, true, 'srcTable exists');

bigquery.dataset(destDataset).table(destTable).exists(
function (err, exists) {
assert.equal(err, null);
assert.equal(exists, true, 'destTable exists');
done();
}
);
}
);
assert.notEqual(rows, null);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're asserting that rows is an array (asserting a type), then I think it's redundant to assert that rows is not null (asserting a type).

assert.equal(Array.isArray(rows), true);
assert.equal(rows.length > 0, true);
assert.equal(console.log.calledOnce, true);
assert.deepEqual(console.log.firstCall.args, ['Found %d row(s)!', rows.length]);
done();
});
});
});
Expand Down
27 changes: 24 additions & 3 deletions bigquery/tables.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,19 @@ function listTables (options, callback) {
}
// [END list_tables]

function listRows (dataset, table, callback) {
var bigquery = BigQuery();
var tableObj = bigquery.dataset(dataset).table(table);
tableObj.getRows(function (err, rows) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a space between tableObj and table.getRows for readability. Also, add the following above tableObject.getRows:

// See https://googlecloudplatform.github.io/google-cloud-node/#/docs/bigquery/latest/bigquery/table?method=getRows

if (err) {
return callback(err);
}

console.log('Found %d row(s)!', rows.length);
return callback(null, rows);
});
}

// [START delete_table]
/**
* Deletes a table with the specified name from the specified dataset.
Expand Down Expand Up @@ -236,6 +249,7 @@ var fs = require('fs');
var program = module.exports = {
createTable: createTable,
listTables: listTables,
listRows: listRows,
deleteTable: deleteTable,
importFile: importFile,
exportTableToGCS: exportTableToGCS,
Expand All @@ -252,7 +266,7 @@ cli
.command('create <dataset> <table>', 'Create a new table in the specified dataset.', {}, function (options) {
program.createTable(utils.pick(options, ['dataset', 'table']), utils.makeHandler());
})
.command('list <dataset>', 'List tables in the specified dataset.', {}, function (options) {
.command('tables <dataset>', 'List tables in the specified dataset.', {}, function (options) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should stay list to be consistent with all the other sample CLIs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the problem is that we can list both tables and rows - should I use something like list_tables and list_rows instead?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking of having list for listing tables, since the sample is tables.js, and browse for the rows, which matches the sample description "Browse a table.".

node tables list my_dataset
node tables browse my_dataset my_table

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or

node tables list my_dataset
node tables rows my_dataset my_table
or
node tables getRows my_dataset my_table
or
node tables get-rows my_dataset my_table
or
node tables listRows my_dataset my_table
or
node tables list-rows my_dataset my_table

I don't really mind what the CLI command is for running listRows, but I think the command for listing tables should be list for consistency.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Point taken, will fix.

program.listTables(utils.pick(options, ['dataset']), utils.makeHandler(true, 'id'));
})
.command('delete <dataset> <table>', 'Delete a table in the specified dataset.', {}, function (options) {
Expand All @@ -270,6 +284,9 @@ cli
);
}
)
.command('rows <dataset> <table>', 'List the rows in a BigQuery table.', {}, function (options) {
program.listRows(options.dataset, options.table, utils.makeHandler());
})
.command('import <dataset> <table> <file>', 'Import data from a local file or a Google Cloud Storage file into BigQuery.', {
bucket: {
alias: 'b',
Expand Down Expand Up @@ -328,9 +345,13 @@ cli
'Create table "my_table" in "my_dataset".'
)
.example(
'node $0 list my_dataset',
'node $0 tables my_dataset',
'List tables in "my_dataset".'
)
.example(
'node $0 rows my_dataset my_table',
'List rows from "my_table" in "my_dataset".'
)
.example(
'node $0 delete my_dataset my_table',
'Delete "my_table" from "my_dataset".'
Expand Down Expand Up @@ -363,7 +384,7 @@ cli
'node $0 copy src_dataset src_table dest_dataset dest_table',
'Copy src_dataset:src_table to dest_dataset:dest_table.'
)
.wrap(100)
.wrap(120)
.recommendCommands()
.epilogue('For more information, see https://cloud.google.com/bigquery/docs');

Expand Down
44 changes: 41 additions & 3 deletions bigquery/test/tables.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,10 @@ function getSample () {
var tableMock = {
export: sinon.stub().yields(null, jobMock),
copy: sinon.stub().yields(null, jobMock),
delete: sinon.stub().yields(null),
import: sinon.stub().yields(null, jobMock),
insert: sinon.stub().yields(null, errorList)
insert: sinon.stub().yields(null, errorList),
getRows: sinon.stub().yields(null, jsonArray),
delete: sinon.stub().yields(null)
};
var datasetMock = {
table: sinon.stub().returns(tableMock),
Expand Down Expand Up @@ -185,6 +186,34 @@ describe('bigquery:tables', function () {
});
});

describe('listRows', function () {
it('should list rows', function () {
var sample = getSample();
var callback = sinon.stub();

sample.program.listRows(dataset, table, callback);

assert.equal(sample.mocks.table.getRows.calledOnce, true);
assert.deepEqual(sample.mocks.table.getRows.firstCall.args.slice(0, -1), []);
assert.equal(callback.calledOnce, true);
assert.deepEqual(callback.firstCall.args, [null, jsonArray]);
assert.equal(console.log.calledOnce, true);
assert.deepEqual(console.log.firstCall.args, ['Found %d row(s)!', jsonArray.length]);
});

it('should handle error', function () {
var error = new Error('error');
var sample = getSample();
var callback = sinon.stub();
sample.mocks.table.getRows.yields(error);

sample.program.listRows(dataset, table, callback);

assert.equal(callback.calledOnce, true);
assert.deepEqual(callback.firstCall.args, [error]);
});
});

describe('deleteTable', function () {
it('should delete a table', function () {
var sample = getSample();
Expand Down Expand Up @@ -395,11 +424,20 @@ describe('bigquery:tables', function () {
var program = getSample().program;
program.listTables = sinon.stub();

program.main(['list', dataset]);
program.main(['tables', dataset]);
assert.equal(program.listTables.calledOnce, true);
assert.deepEqual(program.listTables.firstCall.args.slice(0, -1), [{ dataset: dataset }]);
});

it('should call listRows', function () {
var program = getSample().program;
program.listRows = sinon.stub();

program.main(['rows', dataset, table]);
assert.equal(program.listRows.calledOnce, true);
assert.deepEqual(program.listRows.firstCall.args.slice(0, -1), [dataset, table]);
});

it('should call deleteTable', function () {
var program = getSample().program;
program.deleteTable = sinon.stub();
Expand Down