Skip to content

Conversation

@tseaver
Copy link
Contributor

@tseaver tseaver commented Jun 29, 2016

This is the monster. See #1850 (comment) for the PRs which landed on the bigtable-v2 branch.

tseaver added 30 commits June 24, 2016 15:59
Pass the 'PROTOC_CMD' and 'GRPC_PLUGIN' values through from make's
environment to the scripts used to pick apart GRPC-specific code.
Put them in a separate '_generated_v2' subdir, to ease migration.

Attempt to factor generation process for clarity (toward #1482).
It was never actually implemented on the back-end in V1, and has been
dropped altogether in V2.
Indicate their V1 source in their names.

Prepratory to converting to V2 equivalents.
…ints

Alias Bigtable V1 imports / factories / entry point constants.
Note that {Create,Update,Delete}ColumnFamily messages all collapse to
ModifyColumnFamilies.
Update bigtable.column_family to use V2 protos
Assert that the state is 'ROW_IN_PROGRESS', and check that the completed
rows match the expected results.
Verify that completed, non-error rows match expected results after an
invalid chunk testcase.
…ror-cases

Clarify handling of V2 'ReadRowsResponse' error cases
Folds new ReadRowsResponse logic from #1907, #1915 into table row
handling.
Convert non-instance-admin protos to Bigtable V2
In V2, those operations are on the instance.
…tos.

Closes #1928.

This is a better hack for #1482, but we still really want #1384.
…1 protos.

This is a better hack for #1482, but we still really want #1384.
@tseaver
Copy link
Contributor Author

tseaver commented Jun 29, 2016

Just FTR, I was able to run the bigtable system tests under Python 3.4:

$ tox -re system-tests3 --notest
$ .tox/system-tests3/bin/pip install grpcio 
$ .tox/system-tests3/bin/python system_tests/run_system_test.py --package=bigtable
D0629 13:53:51.485668817   28141 ev_posix.c:101]             Using polling engine: poll
test_read_row (bigtable.TestDataAPI) ... ok
test_read_rows (bigtable.TestDataAPI) ... ok
test_read_with_label_applied (bigtable.TestDataAPI) ... ok
test_create_instance (bigtable.TestInstanceAdminAPI) ... ok
test_list_instances (bigtable.TestInstanceAdminAPI) ... ok
test_reload (bigtable.TestInstanceAdminAPI) ... ok
test_update (bigtable.TestInstanceAdminAPI) ... ok
test_create_column_family (bigtable.TestTableAdminAPI) ... ok
test_create_table (bigtable.TestTableAdminAPI) ... ok
test_delete_column_family (bigtable.TestTableAdminAPI) ... ok
test_list_tables (bigtable.TestTableAdminAPI) ... ok
test_update_column_family (bigtable.TestTableAdminAPI) ... ok
(grpcio shutdown noise elided)

----------------------------------------------------------------------
Ran 12 tests in 29.129s

OK

The bigtable-happybase system tests aren't yet Py3k-clean (they use native string literals for row keys, etc.)

@mbrukman
Copy link
Contributor

What's the status of this PR? @daspecster, are you reviewing it?

@daspecster
Copy link
Contributor

@mbrukman, I am looking it over.
I'm sure we need more eyes than mine on it :)


.. code:: python

intances = client.list_intances()

This comment was marked as spam.

This comment was marked as spam.

@lesv
Copy link

lesv commented Jun 29, 2016

@tswast PTAL

@lesv
Copy link

lesv commented Jun 29, 2016

@jonparrott PTAL - could you review this PR - ideally, we'd like to get it published today as part of Bigtable GA.

* a :class:`Client` owns a :class:`.Cluster`
* a :class:`.Cluster` owns a :class:`Table <gcloud.bigtable.table.Table>`
* a :class:`Client` owns a :class:`.Instance`

This comment was marked as spam.

This comment was marked as spam.

@tswast
Copy link
Contributor

tswast commented Jun 29, 2016

Changes LGTM. I tested with this branch on my Hello World samples, and both ran fine. Thanks!

@tseaver
Copy link
Contributor Author

tseaver commented Jun 29, 2016

@dhermes any objections to a merge?

@lesv
Copy link

lesv commented Jun 29, 2016

@tseaver - We've had two pythonistas review. dhermes is away. It's ok to merge.

@tseaver tseaver merged commit 1a2fa6c into master Jun 29, 2016
@tseaver tseaver deleted the bigtable-v2 branch June 29, 2016 22:25
@lesv
Copy link

lesv commented Jun 29, 2016

@tseaver Can you let us know the PyPi status -- (ie submit and let us know)

@tseaver
Copy link
Contributor Author

tseaver commented Jun 29, 2016

@lesv 0.17.0 is tagged. Travis will push the release when the tag build is finished.

@lesv
Copy link

lesv commented Jun 29, 2016

@tseaver What about docs?

@tseaver
Copy link
Contributor Author

tseaver commented Jun 29, 2016

@lesv RTD already shows the 0.17.0 docs. The github.io docs get updated by the same Travis job that makes the PyPI release.

@mbrukman
Copy link
Contributor

RTD docs on instances have several misspellings: it says intances instead of instances (missing s) which I thought we addressed during code review. One of the more visible ones is right in the section header.

@mbrukman
Copy link
Contributor

mbrukman commented Jun 29, 2016

Also, RTD Bigtable usage docs talk only about clusters, does not mention instances at all.

There's also a possible typo or syntax error on the instance API page, where it says:

[...] object will be returned by create() <gcloud.bigtable.instance.Instance.create>`().

@tseaver
Copy link
Contributor Author

tseaver commented Jun 29, 2016

@mbrukman
Copy link
Contributor

Thank you, @tseaver!! Much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigtable Issues related to the Bigtable API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants