Spanner: Re-generate gapic and drop usage of gax by lukesneeringer · Pull Request #4695 · googleapis/google-cloud-python

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.

Sign in to view

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


Sign in to view

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


Sign in to view

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


Sign in to view

+):
 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 gaxJan 4, 2018

tseaver reviewed Jan 5, 2018
View reviewed changes

Luke Sneeringerand 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 a087a11Compare 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 masterJan 10, 2018

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

parthea pushed a commit that referenced this pull request Mar 9, 2026
Spanner: Re-generate gapic and drop usage of gax (#4695)
8bb0e61

		# limitations under the License.

		"""Google namespace package."""

		# limitations under the License.

		"""Google Cloud namespace package."""

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