bigtable: initial support #1279

callmehiphop · 2016-05-02T19:55:33Z

Closes #722

Add support for Bigtable!

TODOS:

stephenplusplus · 2016-05-02T19:57:46Z

Can you throw up some code examples, or is https://gist.github.com/callmehiphop/7849a495cbb5c8deedf9 still accurate?

callmehiphop · 2016-05-02T20:00:21Z

@stephenplusplus I'll add some code examples, still working on convenience methods. Mostly opening this because of the GrpcService changes involving supporting multiple proto files and supporting gRPC streaming APIs. Feedback around those two particular items is pretty important to me ATM.

stephenplusplus · 2016-05-02T20:09:18Z

@callmehiphop cool, can you call out / briefly explain the changes and how they work together?

lib/bigtable/index.js

+ apiVersion: 'v1',
+ protoServices: {
+ BigtableService: googleProtoFiles.bigtable.v1,
+ BigtableTableService: {


stephenplusplus · 2016-05-02T20:33:55Z

I think it looks okay to me. The language (var names, methods, comments) is the most important thing, making sure things are super-logically named to minimize confusion. Seems like everything is okay, but feel free to make tweaks if you find anywhere that could be more explicit.

callmehiphop · 2016-05-03T15:31:11Z

I believe this was brought up a couple of weeks ago, but it would appear the cluster portion of the API has been decoupled from BigTable and moved to a separate proto file.

I'm not sure if this is something I should be tackling here or if a separate PR is in order.

stephenplusplus · 2016-05-03T15:34:46Z

Ah, interesting. @jgeewax do we want to support the Container Engine API anytime soon? It looks like they're working on a grpc API maybe?

If so, we can create {module:container/cluster}, use it from {module:bigtable/cluster}, and just not document {module:container} until we have filled in the rest of the support for Container.

system-test/bigtable.js

+ });
+
+ it('should insert multiple rows', function(done) {
+ var rows = {


callmehiphop · 2016-05-09T06:08:13Z

@stephenplusplus I started work on the documentation, it's still a ways away from being complete, however it will give you an overview of the API usage.

http://callmehiphop.github.io/gcloud-node/#/docs/master/bigtable

PTAL!

callmehiphop · 2016-05-09T06:19:05Z

Clarifying that when I said PTAL, I meant at the doc examples, I'm still refactoring code.

lib/bigtable/filter.js

+
+/**
+ *
+ */


stephenplusplus · 2016-05-10T21:00:14Z

I'm still learning Bigtable as I go here, so I'm moving a little slower than I'd like in the review-- very sorry about that. I'm tracking some thoughts as I go. I figured instead of blasting you with them at the end, I might as well give you something sooner than later. Sorry again if these are bad or impossible ideas, but... here we go.

`deleteCellsFrom`

Can we remove this and instead add some options right on the Row object?

var row = table.row('Kennedy');

// delete a row
row.delete(function(err, apiResponse) {});

// delete a column family
row.deleteColumnFamily('name', function(err, apiResponse) {});

// delete a column
row.deleteColumn('name:first', function(err, apiResponse) {});

// delete all cells
row.empty(function(err, apiResponse) {});

Then when we need to batch multiple mutations, we can introduce a batch method from the Table instance, that queues the mutations then runs them when exec() is called:

// delete multiple cells from rows
table.batch(function(exec) {
  // delete a column family
  row.deleteColumnFamily('name');

  // delete a column
  row.deleteColumn('name:first');

  // delete all cells
  row.empty();

  // run the deletes
  exec();
}, function(err, apiResponse) {});

`getRows`

At this point, I think there's a lot of scary terms and complexity with the filter builder. I think if we could come up with a more intuitive POJO, it might help:

var options = {
  filter: {
    column: [
      'name:first',
      'life:born'
    ]
  }
};

table.getRows(options)
  .on('data', function(row) {
    // row is a Row object
  });

`insert`

Introducing a save method on the Row object might save us from the verbose object that has to be created for insert currently.

var row = table.row('Lincoln');

row.save({
  name: {
    first: 'Abraham',
    last: 'Lincoln'
  }
}, function(err, apiResponse) {});

If we implement batch, that can support multiple saves to multiple rows.

callmehiphop · 2016-05-11T15:23:45Z

Can we remove this and instead add some options right on the Row object?

Then when we need to batch multiple mutations, we can introduce a batch method from the Table instance, that queues the mutations then runs them when exec() is called

SGTM!

At this point, I think there's a lot of scary terms and complexity with the filter builder. I think if we could come up with a more intuitive POJO, it might help

I'm ok with making it a plain old javascript object, but I'm not sure I agree that it's going to make it any less scary. For very simple filters, like the one in your example, it might make it appear less frightening, however for more complex filters that leverage interleaving and/or conditionals the object will has the potential to be massive.

It might be worth mentioning that going POJO for this means that the order of the keys in the object matters - because this would be the order in which the filters are ran. I could see this being confusing for some.

Introducing a save method on the Row object might save us from the verbose object that has to be created for insert currently.

There's already a Row#put method that does exactly this, I can rename it to save() if you prefer.

stephenplusplus · 2016-05-11T15:26:51Z

for more complex filters that leverage interleaving and/or conditionals the object will has the potential to be massive.

Can you whip one of those up so I can see?

There's already a Row#put

Woops, didn't see that. I do prefer save over put to match our API terminology more closely.

callmehiphop · 2016-05-16T16:34:03Z

Here's what I'm thinking for a filter object

Simple filters

{
  sink: Boolean, // maps to `sink`
  pass: Boolean, // maps to `passAllFilter`
  block: Boolean, // maps to `blockAllFilter`
  time: { // maps to `timestampRangeFilter`
    start: Date,
    end: Date
  },
  labels: Boolean // maps to `applyLabelsTransformer`
}

Family filters

A number of filters can be regular expressions (family filter being one of them). For these I think we could allow for String, RegExp and String[].

{ // maps to `familyNameRegexFilter`
  family: 'name',
  family: /^(name|life)/,
  family: ['name', 'life']
}

Row filters

In its simplest form, we would use a similar pattern to the family name filter.

{ // maps to `rowKeyRegexFilter`
  row: 'lincoln',
  row: /^(lincoln|kennedy)/,
  row: ['lincoln', 'kennedy']
}

For further row options we would expand to an object

{
  row: {
    key: String || RegExp || Array, // maps to `rowKeyRegexFilter`
    sample: String, // maps to `rowSampleFilter`
    cellOffset: Number, // maps to `cellsPerRowOffsetFilter`
    cellLimit: Number // maps to `cellsPerRowLimitFilter`
  }
}

Column filters

In its simplest form, we would use a similar pattern to the family name filter.

{ // maps to `columnQualifierRegexFilter`
  column: 'first',
  column: /^(first|last)/,
  column: ['first', 'last']
}

For further column options we would expand into an object

{
  column: {
    name: String || RegExp || Array, // maps to `columnQualifierRegexFilter`
    cellLimit: Number // maps to `cellsPerColumnLimitFilter`
  }
}

You can also search for a range of columns, which I think should look like

{
  column: { // maps to `columnRangeFilter`
    // shorthand for { value: String }
    start: String,

     // expanded version
    start: {
      value: String,
      inclusive: Boolean
    },

    // shorthand for { value: String }
    end: String,

     // expanded version
    end: {
      value: String,
      inclusive: Boolean
    }
  }
}

Value filters

In its simplest form, we would use a similar pattern to the family name filter.

{ // maps to `valueRegexFilter`
  value: 'Abe',
  value: /(Abe|Abraham)/,
  value: ['Abe', 'Abraham']
}

An expanded version would include a value range filter and an option to strip values from results

{
  value: {
    value: String || RegExp || Array, // maps to `valueRegexFilter`
    start: String, // maps to `valueRangeFilter`
    end: String, // maps to `valueRangeFilter`
    strip: Boolean // maps to `stripValueTransformer`
  }
}

Advanced Filters

There are also interleave and condition filters which use multiple filter objects

{
  interleave: [Filter, Filter],
  condition: {
    test: Filter,
    pass: Filter,
    fail: Filter
  }
}

One caveat of using POJO for filters is that the order of the keys can matter, as the filters will execute in said order.

@stephenplusplus WDYT?

stephenplusplus · 2016-05-16T16:43:35Z

It looks awesome 👍

One caveat of using POJO for filters is that the order of the keys can matter, as the filters will execute in said order.

The user will just have to take care to the order they define their filters in the object-- I actually think this is a nice "feature"; their filters will be executed in the order they're defined. I've never seen Object.keys or a for-in not sorted in the order of the definition of the object... is that the concern?

callmehiphop · 2016-05-16T16:52:21Z

I've never seen Object.keys or a for-in not sorted in the order of the definition of the object... is that the concern?

While I have never seen it happen myself, it does not look like something we should depend on.

So maybe I spoke too soon and we should keep the builder?

stephenplusplus · 2016-05-16T16:55:36Z

Check out http://stackoverflow.com/a/32149345/776934

callmehiphop · 2016-05-16T17:02:32Z

Nice, we should be ok then.

sduskis · 2016-05-17T01:30:08Z

Are you aware that Cloud Bigtable is moving to a v2 API? A new node.js implementation should probably use v2 instead of v1. We should talk offline about timelines.

dhermes · 2016-05-17T01:43:27Z

@sduskis I'd love to hear about v2 as well for gcloud-python

system-test/bigtable.js

+ });
+
+ it('should delete a column family', function(done) {
+ FAMILY.delete(function(err) {


stephenplusplus · 2016-07-11T17:18:01Z

One lone change left, then I'll wait out the tests, and welcome Bigtable to the gcloud-node Family of APIs.

coveralls · 2016-07-11T17:48:36Z

Coverage remained the same at 100.0% when pulling ccd85bd on callmehiphop:bigtable into 0a6da60 on GoogleCloudPlatform:master.

stephenplusplus · 2016-07-11T18:26:24Z

CLAs are signed, but unable to verify author consent

I'm cool with you squashing these down to one commit... that might remedy this oddity.

add support for scan filters add examples via jsdoc add bigtable to docs site

README.md

+// Update a row in your table.
+var row = table.row('alincoln');
+
+row.save('follows:gwashington', 1, function(err) {});


coveralls · 2016-07-11T20:18:18Z