jonathansudhakar1 commented on issue #21531: URL: https://github.com/apache/beam/issues/21531#issuecomment-6773835203
This still applies on current master (2d1ac62f7a6) with the client version Beam resolves today, google-cloud-spanner 6.121.0. - `SpannerAccessor.createSpannerOptions` always calls `builder.setHost(spannerConfig.getHostValue())` ([SpannerAccessor.java#L262](https://github.com/apache/beam/blob/2d1ac62f7a6/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java#L262)). The host defaults to `https://batch-spanner.googleapis.com/` ([SpannerConfig.java#L49](https://github.com/apache/beam/blob/2d1ac62f7a6/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java#L49)). - In 6.121.0, `SpannerOptions.Builder.setHost(...)` still ends with `setEmulatorHost(null)` (checked with `javap`), so the emulator host the builder read from `SPANNER_EMULATOR_HOST` is dropped. - Beam only sets an emulator host when `SpannerConfig.withEmulatorHost(...)` is used ([SpannerAccessor.java#L290-L298](https://github.com/apache/beam/blob/2d1ac62f7a6/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java#L290-L298)). A throwaway JUnit test in the `spanner` package (not committed), run with `SPANNER_EMULATOR_HOST=localhost:9010 ./gradlew :sdks:java:io:google-cloud-platform:test --tests ...`, printed: ``` client SpannerOptions.newBuilder().setProjectId("p").build().getHost() -> http://localhost:9010 SpannerAccessor.buildSpannerOptions(SpannerConfig.create()...) -> https://batch-spanner.googleapis.com/ ... with .withEmulatorHost(StaticValueProvider.of("localhost:9010")) -> http://localhost:9010 ``` So `withEmulatorHost(...)` is still the workaround. @nielm, since you last reworked the host defaulting (#34601), would a fix along these lines be welcome? - In `createSpannerOptions`, when `getEmulatorHost()` is unset, no experimental host is set, and the host is the default, fall back to `SPANNER_EMULATOR_HOST`. Then apply the same `setEmulatorHost` plus `NoCredentials` handling as for an explicit emulator host. - An explicit `withEmulatorHost`, or a non-default `withHost`, keeps winning. That matches the client library, where an explicit `setHost` clears the env-derived emulator host. - Testing: read the environment through a package-private hook, so `SpannerAccessorTest` can cover the env-set, explicit-host and explicit-emulator cases without a real environment variable. This is a behavior change for anyone who has `SPANNER_EMULATOR_HOST` set but relies on Beam ignoring it, so I'd like your view before sending a PR. If you'd rather keep requiring `withEmulatorHost`, a Javadoc note on `SpannerConfig` saying the variable is ignored would also close the gap. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
