Design Review

[dhermes]

Just got off video chat with @tseaver and had some notes:

• Why not just pass a different Client (instead of timeout_seconds) to API methods
• For the Row() factory, need to document filter argument 40b04fd
• Needed: Long-form about composing filters (like the one about GC rules: at beginning of Column Families doc) (Adding Bigtable docs. googleapis/google-cloud-python#1547)
• Should we unify commit() and commit_modifications() on Row() into one method?
• Worth using Row() as a context manager? (With Row.commit() on exit.)
• Should Row.delete_cells() just default to Row.ALL_COLUMNS? (Is this dangerous to offer as a default?)
• Would using properties on Row() for conditional mutations make things easier? E.g. Row.true_muts.set_cell() instead of Row.set_cell(..., state=True).

[dhermes]

@tseaver Have you had a chance to review the source yet? I'd love to start the process of moving this into gcloud-python main source (or maybe just making a firm decision on where it should live). The gRPC core library is finally apt-get install-able (sort of), so we are in a great place!

[tseaver]

I ran out of time to look at source before jumping on the road today. I plan to be in review-only mode this coming week, at least a couple of hours a day.

[dhermes]

Review as in PRs? Or does review also encompass reviewing this repo's code in full?

[tseaver]

Dunno. I imagine we could be working out adding bigtable as a subpackage of gcloud. If staging it over as a set of PRs seems like a reasonable plan, that would fit our existing workflow. Otherwise, we would need to figure out how the review would be done in your repository.

[dhermes]

We need a stable way to install the grpc core on Travis before we can bring them over.

The only modules to inspect are:

gcloud_bigtable/__init__.py
gcloud_bigtable/_helpers.py
gcloud_bigtable/_logging.py
gcloud_bigtable/client.py
gcloud_bigtable/cluster.py
gcloud_bigtable/column_family.py
gcloud_bigtable/row.py
gcloud_bigtable/row_data.py
gcloud_bigtable/table.py

and three of those are quite short.

---

The make generate rule for re-building the generated modules should also be reviewed.

At the very least I should put it in a bash file.

---

We could do a final branch or some such thing in this repo and I could do PRs into it?

[dhermes]

I just realized while talking to @jgeewax that 100% of the unit tests can be run without gRPC installed, so it would just be the system tests. w00t!

I'll try to bring them over and somehow garden this "fork" while the other one gets up to speed.