Spanner: Re-generate gapic and drop usage of gax #4695

lukesneeringer · 2018-01-04T19:04:32Z

No description provided.

theacodes · 2018-01-04T19:04:59Z

Towards #4692

theacodes · 2018-01-04T19:11:31Z

@lukesneeringer for co-located generated code, we need to make this line:

_GAPIC_LIBRARY_VERSION = pkg_resources.get_distribution(
    'google-cloud-spanner-admin-database', ).version

into a template so we can make it say:

_GAPIC_LIBRARY_VERSION = pkg_resources.get_distribution(
    'google-cloud-spanner', ).version

Will manually fix for the time being. Do you wanna file a toolkit bug for it?

theacodes · 2018-01-04T19:17:08Z

Also note that the generator clobbered spanner_v1/__init__.py, see 2954120.

theacodes · 2018-01-04T19:26:20Z

Additionally, the autogenerated tests failed to account for the true location of SpannerClient because it's not surfaced in google.cloud.spanner_v1. See 129b26d

lukesneeringer · 2018-01-04T21:00:28Z

Will manually fix for the time being. Do you wanna file a toolkit bug for it?

I think that might be configurable already. I will check.

api_core/google/api_core/page_iterator.py

        super(HTTPIterator, self).__init__(
            client, item_to_value, page_token=page_token,
            max_results=max_results)
+        self.client = client


spanner/google/__init__.py

-# limitations under the License.
-
-"""Google namespace package."""
-


spanner/google/cloud/__init__.py

-# limitations under the License.
-
-"""Google Cloud namespace package."""
-


spanner/google/cloud/spanner_admin_instance_v1/types.py

+):
    for name, message in get_messages(module).items():
        message.__module__ = 'google.cloud.spanner_admin_instance_v1.types'
        setattr(sys.modules[__name__], name, message)


theacodes · 2018-01-05T22:23:59Z

@lukesneeringer this should be ready to review and merge. While the changes to the library itself are minor, there had to be a lot of changes to the tests to remove gax as they were extremely tightly coupled.

theacodes · 2018-01-05T23:06:16Z

@lukesneeringer seems the generator spit out a bad docstring, see 78dc4b0.

lukesneeringer · 2018-01-08T14:46:27Z

@lukesneeringer seems the generator spit out a bad docstring, see 78dc4b0.

My guess is that is the Markdown to Restructed Text conversion that we do, meaning it is a bug in pandoc (or pypandoc).

I will attempt to prove this and then file a bug with them.

theacodes · 2018-01-08T17:06:57Z

I will attempt to prove this and then file a bug with them.

In the meantime, can you file a bug on toolkit for tracking?

lukesneeringer · 2018-01-08T17:14:24Z

googleapis/gapic-generator#1786

theacodes · 2018-01-08T17:27:03Z

@lukesneeringer (cc @dhermes) so I've ran into something and I really don't know how to fix it.

The protos contain docstrings that are incompatible with autodoc.

Warning, treated as error:
/Users/jonwayne/workspace/google-cloud-python/.nox/docs/lib/python3.6/site-packages/google/cloud/spanner_v1/types.py:docstring of google.cloud.spanner_v1.types.TransactionOptions:2:Unexpected section title.

Transactions
============

Source string here: https://github.com/GoogleCloudPlatform/google-cloud-python/blob/a44f6ceff101b338ae5c3e1718b378887ec6fd12/spanner/google/cloud/spanner_v1/proto/transaction_pb2.py#L372

This isn't something I can easily just edit back in place - there's entire prose docs in the proto. What should we do here?

dhermes · 2018-01-08T17:28:48Z

there's entire prose docs in the proto. What should we do here?

I've run into this before and just edited the _pb2 file. AFAIK there is no "fast" / easy fix.

theacodes · 2018-01-08T17:33:09Z

I this case I'd need to remove the entire thing.

lukesneeringer · 2018-01-08T21:47:16Z

This isn't something I can easily just edit back in place - there's entire prose docs in the proto. What should we do here?

I actually copied that entire doc to an entirely-separate RST file, replaced that huge section of the docstring with a reference, and gave Spanner a bug to please change it (internal: 65243734).

theacodes · 2018-01-08T22:08:13Z

Thanks, @lukesneeringer.

We should be good to review and merge after 8e11b2d

theacodes · 2018-01-09T17:43:39Z

@lukesneeringer gentle ping - this should be ready to merge.

lukesneeringer · 2018-01-10T21:36:54Z

Agreed.

lukesneeringer requested a review from theacodes January 4, 2018 19:04

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 4, 2018

theacodes added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jan 4, 2018

lukesneeringer requested a review from tseaver as a code owner January 4, 2018 19:06

theacodes changed the title ~~Spanner: Re-generate autogen.~~ Spanner: Re-generate gapic and drop usage of gax Jan 4, 2018

tseaver reviewed Jan 5, 2018

View reviewed changes

Luke Sneeringer and others added 19 commits January 5, 2018 13:00

Spanner: Re-generate autogen.

1411ad4

Remove gax references from spanner

16d7bc8

Drop gax from setup.py

d7eafa5

Restore clobbered spanner_v1/__init__.py

2f88a7d

Fix version determination in autogenerated packages

3048af7

Fix import errors

eba97de

Fix bad import in autogenerated tests

f469d95

Manually add metadata kwarg to transaction RPCs

625f341

Fix transaction tests

9bea231

Manually add metadata kwarg to read/query RPCs

e1495c9

Fix snapshot tests

2265a7f

Fix session tests

a7f2691

Manually add metadata kwarg to database admin methods

b774f06

Fix instance tests

696526d

Fix batch tests

393da88

Fix database tests

54f3394

Fix client tests

77e2a78

Fix helper tests

26ec6d7

Use public item_to_value property

a087a11

theacodes force-pushed the spanner-regen branch from 3ce81f1 to a087a11 Compare January 5, 2018 21:02

Jon Wayne Parrott added 4 commits January 5, 2018 13:03

Revert changes to api_core

452b4db

Add missing metadata kwargs to several gapic methods

d28fa1a

Fix system tests

5c4874a

Fix remaining unit tests and coverage

1b34751

theacodes added api: spanner Issues related to the Spanner API. and removed do not merge Indicates a pull request not ready for merge, due to either quality or timing. labels Jan 5, 2018

This was referenced Jan 5, 2018

Drop usage of gax in favor of api_core #4692

Closed

Spanner v1.0.0 release #4710

Closed

Jon Wayne Parrott added 2 commits January 5, 2018 14:42

Fix Unavailable -> ServiceUnavailable

c05a962

Fix bad docstring

78dc4b0

Fix some more bad docstrings

a44f6ce

tseaver mentioned this pull request Jan 8, 2018

Mutating '__module__' of proto-generated messages considered harmful #4715

Closed

Re-move narrative transaction doc to stand-alone RST file.

8e11b2d

lukesneeringer merged commit d2ff0b9 into master Jan 10, 2018

lukesneeringer deleted the spanner-regen branch January 10, 2018 21:38

		# limitations under the License.

		"""Google namespace package."""

		# limitations under the License.

		"""Google Cloud namespace package."""

Spanner: Re-generate gapic and drop usage of gax #4695

Spanner: Re-generate gapic and drop usage of gax #4695

Uh oh!

Conversation

lukesneeringer commented Jan 4, 2018

Uh oh!

theacodes commented Jan 4, 2018

Uh oh!

theacodes commented Jan 4, 2018

Uh oh!

theacodes commented Jan 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theacodes commented Jan 4, 2018

Uh oh!

lukesneeringer commented Jan 4, 2018

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!

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!

This comment was marked as spam.

Uh oh!

theacodes commented Jan 5, 2018

Uh oh!

theacodes commented Jan 5, 2018

Uh oh!

lukesneeringer commented Jan 8, 2018

Uh oh!

theacodes commented Jan 8, 2018

Uh oh!

lukesneeringer commented Jan 8, 2018

Uh oh!

theacodes commented Jan 8, 2018

Uh oh!

dhermes commented Jan 8, 2018

Uh oh!

theacodes commented Jan 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lukesneeringer commented Jan 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theacodes commented Jan 8, 2018

Uh oh!

theacodes commented Jan 9, 2018

Uh oh!

lukesneeringer commented Jan 10, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

theacodes commented Jan 4, 2018 •

edited

Loading

theacodes commented Jan 8, 2018 •

edited

Loading

lukesneeringer commented Jan 8, 2018 •

edited

Loading