BigTable: Integration of GAPIC by aneepct · Pull Request #5178 · googleapis/google-cloud-python

aneepct · 2018-04-11T13:34:58Z

Full integration of GAPIC code into BigTable including GAPIC admin api.

…thub.com/zakons/google-cloud-python into feature/gapic

…into feature/read_rows_retry

…into feature/gapic

zakons · 2018-04-23T04:25:56Z

@jonparrott OK, now this PR is REALLY ready to merge. Cluster was taken out and now it was put back in, so the original client is now fully restored, with the added GAPIC API's hidden underneath. Please proceed with the merge.

theacodes

Looks mostly good, a few nits and this is mergable.

Sign in to view

-    :type user_agent: str
-    :param user_agent: (Optional) The user agent to be used with API request.
-                       Defaults to :const:`DEFAULT_USER_AGENT`.
+    :type project: :instance: grpc.Channel


Sign in to view

+        self._credentials = credentials
        self.SCOPE = self._get_scopes()

        # NOTE: This API has no use for the _http argument, but sending it


Sign in to view

-                  :class:`~google.cloud.bigtable.instance.Instance` objects
-                  returned and the second is a list of strings (the failed
-                  locations in the request).
+        :rtype: :class:`~google.gax.PageIterator`


Sign in to view

            before calling :meth:`create`.

-        :rtype: :class:`Operation`
+        :rtype: :class:`~google.cloud.bigtable_admin_v2.types._OperationFuture`


Sign in to view

-        :rtype: :class:`Operation`
-        :returns: The long-running operation corresponding to the
-                  create operation.
+        :rtype: :class:`~google.cloud.bigtable_admin_v2.types._OperationFuture`


zakons · 2018-04-30T03:18:35Z

@jonparrott Sorry, it seems github did not send me a notification email when you last left review comments, even though I am subscribed to updates on this pull request.

The requested updates are done now. If possible, please add our @ names when commenting as this DOES work for email notifications and will allow us to respond much more quickly.

After this PR is merged, we will pull from master into PR #5113 and ready that one to merge. We are trying to stay in sync with your changes so we can move this along more quickly. Thanks!

…into feature/gapic

theacodes

Almost there.

Sign in to view

-from google.cloud.bigtable.client import Client

-
-__all__ = ['__version__', 'Client']


Sign in to view

+        self.project = project
        self._read_only = bool(read_only)
        self._admin = bool(admin)
+        self.channel = channel


Sign in to view

-            self._instance_stub_internal = _make_instance_stub(self)
-            self._operations_stub_internal = _make_operations_stub(self)
-            self._table_stub_internal = _make_table_stub(self)
+        if self.channel is not None:


theacodes · 2018-05-01T19:27:32Z

@zakons I reviewed, PTAL.

zakons · 2018-05-02T04:24:01Z

@jonparrott Thanks for a very thorough review. Hoping this is ready! ;-)

zakons · 2018-05-03T00:19:05Z

@jonparrott Many thanks. This was a big merge.
Please review PR #5275 next as it is prioritized for the Python BigTable beta release.

crwilcox · 2018-05-04T22:34:29Z

@zakons and @sduskis
Currently master is not passing tests for BigTable. They are failing because project isn't being set. It seems in the rework Client stopped inheriting from ClientWithProject and now we aren't auto-magically setting the user's project anymore and instead are assuming the project name is 'None'.

If you make the following modifications to google/cloud/bigtable/client.py

+from google.cloud.client import ClientWithProject

 from google.cloud import bigtable_v2
 from google.cloud import bigtable_admin_v2
@@ -44,7 +45,7 @@ READ_ONLY_SCOPE = 'https://www.googleapis.com/auth/bigtable.data.readonly'
 """Scope for reading table data."""


-class Client(object):
+class Client(ClientWithProject):

You get further, but the next thing you will get is a failure on 43 tests about ValueError: The channelandcredentials arguments to BigtableClient are mutually exclusive.

zakons · 2018-05-07T03:12:21Z

@crwilcox These system tests are being fixed right now and will let you and @jonparrott know as soon as a PR is ready with this.

Luke Sneeringer and others added 30 commits January 4, 2018 10:29

Provide new auto-generated layer for Bigtable.

3edca4c

Merge branch 'master' into bigtable-regen

a677959

Add retry for read rows

d540349

Fix line lengths

4aa5702

Add parameter start_inclusive to _create_row_request

f7d4c69

Update unit test case for yield rows retry

28892ce

Fix line lengths

8d141f0

Edit table.py as requested changes

902f11b

Added code review changes from @jonparrott.

4afabb1

Fix parameter name on _RetryableReadRows class

79698b1

Add retry for Deadline Exceeded on read rows

9b778be

Fix line over-indented

cafc5ad

Fix line over-indented

f974752

Merge commit '3edca4c3299e85d4346dae49232d540a2d34002c' of https://gi…

f7242f4

…thub.com/zakons/google-cloud-python into feature/gapic

Merge commit 'a67795951bc9f490265eda7cf511e17d2cc70921' of https://gi…

180d1d5

…thub.com/zakons/google-cloud-python into feature/gapic

Working on gapic integration

f2e39e0

Refactore yield_rows retry

13adfd6

1. Moved last_scanned_row_key into YieldRowsData.

ca69a2f

Fix line over-indented

02c154e

Fix row_data.py

b6da7fa

Change row.py as requested

f1a2d92

Create new test case for yield_rows iterator failure

65c1c4b

Fix for start_inclusive on retry read rows.

170a173

Merge branch 'master' of https://github.com/zakons/google-cloud-python …

5da3fd3

…into feature/read_rows_retry

Change bigtable_pb2 imports to use from gapic library.

f7a58b5

Merge branch 'feature/read_rows_retry' of https://github.com/zakons/g…

c8f664b

…oogle-cloud-python into feature/gapic

Working on gapic

d5f0d98

Integrate bigtable GAPIC client

32df7bd

Refactoring unit tests

77ec950

Refactor to use GAPIC client

efbf2f5

Aneep Tandel added 7 commits April 19, 2018 14:49

Fix the doc string on list_instances

a74ff23

Fix Definition list ends without a blank line

e7c64ae

Remove custer doc from docs

a8d6f83

Added cluster

b67ab6a

Add cluster.rst bigtable docs

634f3b4

Merge branch 'master' of https://github.com/zakons/google-cloud-python …

480f26b

…into feature/gapic

Add cluster in toctree

a9b6d90

theacodes suggested changes Apr 23, 2018

View reviewed changes

aneepct added 2 commits April 30, 2018 07:33

Fix required changes on doc strings

ab91e64

Fix required changes on doc strings

8bb0816

Merge branch 'master' of https://github.com/zakons/google-cloud-python …

3702901

…into feature/gapic

theacodes mentioned this pull request May 1, 2018

BigTable: row.commit throwing _Rendezvous exception instead of ServiceUnavailable #4656

Closed

zakons mentioned this pull request May 1, 2018

BigTable: Add data app profile id #5275

Closed

theacodes suggested changes May 1, 2018

View reviewed changes

Fix requested review changes

8c29ca6

aneepct mentioned this pull request May 2, 2018

BigTable: Add split keys on create table #5283

Closed

theacodes approved these changes May 2, 2018

View reviewed changes

theacodes merged commit 86102c9 into googleapis:master May 2, 2018

sduskis mentioned this pull request May 4, 2018

Bigtable: Auto-gen update. #4693

Closed

aneepct mentioned this pull request May 23, 2018

BigTable: Add split keys on create table #5370

Closed

tseaver mentioned this pull request Jul 17, 2018

Retry temporary failures for Bigtable scans #2255

Closed

nkemnitz mentioned this pull request Oct 23, 2018

Bigtable: ConditionalRow not passing parameters to CheckAndMutateRowRequest #6292

Closed

parthea pushed a commit that referenced this pull request Nov 22, 2025

Integrate new generated low-level client (#5178)

05375b3

		from google.cloud.bigtable.client import Client


		__all__ = ['__version__', 'Client']

Conversation

aneepct commented Apr 11, 2018

Uh oh!

zakons commented Apr 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theacodes left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

zakons commented Apr 30, 2018

Uh oh!

theacodes left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

theacodes commented May 1, 2018

Uh oh!

zakons commented May 2, 2018

Uh oh!

zakons commented May 3, 2018

Uh oh!

crwilcox commented May 4, 2018

Uh oh!

zakons commented May 7, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

zakons commented Apr 23, 2018 •

edited

Loading