feat(spanner): split GAPIC from the spanner handwritten package#8931
feat(spanner): split GAPIC from the spanner handwritten package#8931quirogas wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request replaces local proto files and generated clients with the external '@google-cloud/spanner-api' package, updating import paths across the codebase, removing obsolete generator scripts, and updating package dependencies. The review feedback suggests importing 'protos' directly from the public entry point of '@google-cloud/spanner-api' instead of deep-importing, simplifying the client re-exports in 'src/index.ts', and using a cleaner direct require path for 'status.json' from 'google-gax' in the mock server tests.
| * Reference to {@link v1.SpannerClient} | ||
| */ | ||
| import * as protos from '../protos/protos'; | ||
| import * as protos from '@google-cloud/spanner-api/build/protos/protos'; |
There was a problem hiding this comment.
Instead of deep-importing from the internal build/ directory of @google-cloud/spanner-api, you can import protos directly from the public entry point of the package. This is safer, more maintainable, and avoids potential module resolution issues if the package's internal structure changes or if strict package exports are enforced.
| import * as protos from '@google-cloud/spanner-api/build/protos/protos'; | |
| import {protos} from '@google-cloud/spanner-api'; |
| const DatabaseAdminClient = v1.DatabaseAdminClient; | ||
| type DatabaseAdminClient = v1.DatabaseAdminClient; | ||
| const InstanceAdminClient = v1.InstanceAdminClient; | ||
| type InstanceAdminClient = v1.InstanceAdminClient; | ||
| const SpannerClient = v1.SpannerClient; | ||
| type SpannerClient = v1.SpannerClient; | ||
| export { DatabaseAdminClient, InstanceAdminClient, SpannerClient }; |
There was a problem hiding this comment.
These lines are highly verbose and redundant. In TypeScript, you can directly re-export both the values and types of these clients from @google-cloud/spanner-api in a single statement. Additionally, this ensures that SpannerExecutorProxyClient is also re-exported at the top level, as mentioned in the pull request description.
| const DatabaseAdminClient = v1.DatabaseAdminClient; | |
| type DatabaseAdminClient = v1.DatabaseAdminClient; | |
| const InstanceAdminClient = v1.InstanceAdminClient; | |
| type InstanceAdminClient = v1.InstanceAdminClient; | |
| const SpannerClient = v1.SpannerClient; | |
| type SpannerClient = v1.SpannerClient; | |
| export { DatabaseAdminClient, InstanceAdminClient, SpannerClient }; | |
| export { DatabaseAdminClient, InstanceAdminClient, SpannerClient, SpannerExecutorProxyClient } from '@google-cloud/spanner-api'; |
| const protobufjs = require('protobufjs'); | ||
| const statusProtoJson = require(path.join(path.dirname(require.resolve('google-gax')), '../protos/status.json')); | ||
| const RetryInfo: any = protobufjs.Root.fromJSON(statusProtoJson).lookupType('google.rpc.RetryInfo'); |
There was a problem hiding this comment.
Instead of using a fragile combination of require.resolve, path.dirname, and path.join to locate status.json inside google-gax, you can require it directly as 'google-gax/protos/status.json'. This is much cleaner, more readable, and less prone to breaking if the internal file structure of google-gax changes.
| const protobufjs = require('protobufjs'); | |
| const statusProtoJson = require(path.join(path.dirname(require.resolve('google-gax')), '../protos/status.json')); | |
| const RetryInfo: any = protobufjs.Root.fromJSON(statusProtoJson).lookupType('google.rpc.RetryInfo'); | |
| const protobufjs = require('protobufjs'); | |
| const statusProtoJson = require('google-gax/protos/status.json'); | |
| const RetryInfo: any = protobufjs.Root.fromJSON(statusProtoJson).lookupType('google.rpc.RetryInfo'); |
|
Closing this one as a dup for #8927, integrated some bits of it! |
Decouple the handwritten Spanner wrapper from the auto-generated GAPIC client layer by importing a standalone version of the GAPIC library. Standalone
-apipackages are now generated and maintained independently.src/v1/) and OwlBot files (owlbot.py,.OwlBot.yaml).@google-cloud/spanner-api(^0.2.0).v1, top-level sub-clients (SpannerClient,DatabaseAdminClient,InstanceAdminClient,SpannerExecutorProxyClient), andprotosfrom@google-cloud/spanner- api.protobufjsstatus definition loading in mock server test utilities.Internal: b/531797111