You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/02/14 07:04:36 UTC

[GitHub] [beam] zoercai opened a new pull request #16846: test: Add sad path tests for Cloud Spanner's ReadChangeStream

zoercai opened a new pull request #16846:
URL: https://github.com/apache/beam/pull/16846


   Add sad path unit tests for the Cloud Spanner's ReadChangeStream, including testing for errors from the Spanner backend.


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] thiagotnunes commented on a change in pull request #16846: test: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
thiagotnunes commented on a change in pull request #16846:
URL: https://github.com/apache/beam/pull/16846#discussion_r806467293



##########
File path: sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/SpannerChangeStreamErrorTest.java
##########
@@ -0,0 +1,494 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.spanner.changestreams;
+
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_CREATED_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_END_TIMESTAMP;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_FINISHED_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_HEARTBEAT_MILLIS;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_PARENT_TOKENS;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_PARTITION_TOKEN;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_RUNNING_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_SCHEDULED_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_START_TIMESTAMP;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_STATE;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_WATERMARK;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.google.api.gax.grpc.testing.MockServiceHelper;
+import com.google.api.gax.retrying.RetrySettings;
+import com.google.cloud.Timestamp;
+import com.google.cloud.spanner.ErrorCode;
+import com.google.cloud.spanner.MockSpannerServiceImpl;
+import com.google.cloud.spanner.MockSpannerServiceImpl.SimulatedExecutionTime;
+import com.google.cloud.spanner.MockSpannerServiceImpl.StatementResult;
+import com.google.cloud.spanner.Statement;
+import com.google.protobuf.ListValue;
+import com.google.protobuf.NullValue;
+import com.google.protobuf.Value;
+import com.google.spanner.v1.ExecuteSqlRequest;
+import com.google.spanner.v1.ResultSet;
+import com.google.spanner.v1.ResultSetMetadata;
+import com.google.spanner.v1.StructType;
+import com.google.spanner.v1.StructType.Field;
+import com.google.spanner.v1.Type;
+import com.google.spanner.v1.TypeCode;
+import io.grpc.Status;
+import java.io.Serializable;
+import java.util.Collections;
+import org.apache.beam.sdk.Pipeline.PipelineExecutionException;
+import org.apache.beam.sdk.io.gcp.spanner.SpannerConfig;
+import org.apache.beam.sdk.io.gcp.spanner.SpannerIO;
+import org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.DaoFactory;
+import org.apache.beam.sdk.io.gcp.spanner.changestreams.model.PartitionMetadata.State;
+import org.apache.beam.sdk.options.ValueProvider.StaticValueProvider;
+import org.apache.beam.sdk.testing.TestPipeline;
+import org.apache.beam.vendor.guava.v26_0_jre.com.google.common.collect.ImmutableSet;
+import org.hamcrest.Matchers;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public class SpannerChangeStreamErrorTest implements Serializable {
+
+  public static final String SPANNER_HOST = "my-host";
+  private static final String TEST_PROJECT = "my-project";
+  private static final String TEST_INSTANCE = "my-instance";
+  private static final String TEST_DATABASE = "my-database";
+  private static final String TEST_TABLE = "my-metadata-table";
+  private static final String TEST_CHANGE_STREAM = "my-change-stream";
+
+  @Rule
+  public final transient TestPipeline pipeline =
+      TestPipeline.create().enableAbandonedNodeEnforcement(false);
+
+  @Rule public final transient ExpectedException thrown = ExpectedException.none();
+
+  private MockSpannerServiceImpl mockSpannerService;
+  private MockServiceHelper serviceHelper;
+
+  @Before
+  public void setUp() throws Exception {
+    mockSpannerService = new MockSpannerServiceImpl();
+    serviceHelper =
+        new MockServiceHelper(SPANNER_HOST, Collections.singletonList(mockSpannerService));
+    serviceHelper.start();
+    serviceHelper.reset();
+  }
+
+  @After
+  public void tearDown() throws NoSuchFieldException, IllegalAccessException {
+    serviceHelper.reset();
+    serviceHelper.stop();
+    mockSpannerService.reset();
+    resetDaoFactoryFields();
+  }
+
+  @Test
+  public void testResourceExhaustedDoesNotRetry() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofStickyException(Status.RESOURCE_EXHAUSTED.asRuntimeException()));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.RESOURCE_EXHAUSTED.name());
+    }
+  }
+
+  @Test
+  public void testUnavailableExceptionRetries() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofExceptions(
+            ImmutableSet.of(
+                Status.UNAVAILABLE.asRuntimeException(),
+                Status.RESOURCE_EXHAUSTED.asRuntimeException())));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      assertThat(
+          mockSpannerService.countRequestsOfType(ExecuteSqlRequest.class), Matchers.greaterThan(1));
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.RESOURCE_EXHAUSTED.name());
+    }
+  }
+
+  @Test
+  public void testAbortedExceptionRetries() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofStickyException(Status.ABORTED.asRuntimeException()));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      assertThat(
+          mockSpannerService.countRequestsOfType(ExecuteSqlRequest.class), Matchers.greaterThan(1));
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.ABORTED.name());
+    }
+  }
+
+  @Test
+  public void testUnknownExceptionDoesNotRetry() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofStickyException(Status.UNKNOWN.asRuntimeException()));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      assertThat(
+          mockSpannerService.countRequestsOfType(ExecuteSqlRequest.class), Matchers.equalTo(1));
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.UNKNOWN.name());
+    }
+  }
+
+  @Test
+  public void testInvalidRecordReceived() {
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+
+    mockTableExists();
+    ResultSet getPartitionResultSet = mockGetParentPartition(now, after3Seconds);
+    mockGetWatermark(now);
+    mockGetPartitionsAfter(
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds(), now.getNanos() + 1000),
+        getPartitionResultSet);
+    mockGetPartitionsAfter(
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds(), now.getNanos() - 1000),
+        getPartitionResultSet);
+
+    Statement changeStreamQueryStatement =
+        Statement.newBuilder(
+                "SELECT * FROM READ_my-change-stream(   start_timestamp => @startTimestamp,   end_timestamp => @endTimestamp,   partition_token => @partitionToken,   read_options => null,   heartbeat_milliseconds => @heartbeatMillis)")

Review comment:
       nit: we could remove the extra spacing here (probably spotless added it when merging lines into one)

##########
File path: buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy
##########
@@ -454,8 +454,12 @@ class BeamModulePlugin implements Plugin<Project> {
     def checkerframework_version = "3.10.0"
     def classgraph_version = "4.8.104"
     def errorprone_version = "2.10.0"
+    // Try to keep gax_version consistent with gax-grpc version in google_cloud_platform_libraries_bom
+    def gax_version = "2.8.1"

Review comment:
       I think that this one will work without the version as the BOM defines a version for it (https://mvnrepository.com/artifact/com.google.cloud/libraries-bom/24.2.0)

##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java
##########
@@ -107,34 +108,56 @@ public static SpannerAccessor getOrCreate(SpannerConfig spannerConfig) {
   private static SpannerAccessor createAndConnect(SpannerConfig spannerConfig) {
     SpannerOptions.Builder builder = SpannerOptions.newBuilder();
 
-    ValueProvider<Duration> commitDeadline = spannerConfig.getCommitDeadline();
-    if (commitDeadline != null && commitDeadline.get().getMillis() > 0) {
+    // Set retryable codes
+    if (spannerConfig.getRetryableCodes() != null) {

Review comment:
       Makes sense, thanks for trying. I think this approach is nice.

##########
File path: sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/SpannerChangeStreamErrorTest.java
##########
@@ -0,0 +1,494 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.spanner.changestreams;
+
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_CREATED_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_END_TIMESTAMP;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_FINISHED_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_HEARTBEAT_MILLIS;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_PARENT_TOKENS;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_PARTITION_TOKEN;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_RUNNING_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_SCHEDULED_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_START_TIMESTAMP;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_STATE;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_WATERMARK;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.google.api.gax.grpc.testing.MockServiceHelper;
+import com.google.api.gax.retrying.RetrySettings;
+import com.google.cloud.Timestamp;
+import com.google.cloud.spanner.ErrorCode;
+import com.google.cloud.spanner.MockSpannerServiceImpl;
+import com.google.cloud.spanner.MockSpannerServiceImpl.SimulatedExecutionTime;
+import com.google.cloud.spanner.MockSpannerServiceImpl.StatementResult;
+import com.google.cloud.spanner.Statement;
+import com.google.protobuf.ListValue;
+import com.google.protobuf.NullValue;
+import com.google.protobuf.Value;
+import com.google.spanner.v1.ExecuteSqlRequest;
+import com.google.spanner.v1.ResultSet;
+import com.google.spanner.v1.ResultSetMetadata;
+import com.google.spanner.v1.StructType;
+import com.google.spanner.v1.StructType.Field;
+import com.google.spanner.v1.Type;
+import com.google.spanner.v1.TypeCode;
+import io.grpc.Status;
+import java.io.Serializable;
+import java.util.Collections;
+import org.apache.beam.sdk.Pipeline.PipelineExecutionException;
+import org.apache.beam.sdk.io.gcp.spanner.SpannerConfig;
+import org.apache.beam.sdk.io.gcp.spanner.SpannerIO;
+import org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.DaoFactory;
+import org.apache.beam.sdk.io.gcp.spanner.changestreams.model.PartitionMetadata.State;
+import org.apache.beam.sdk.options.ValueProvider.StaticValueProvider;
+import org.apache.beam.sdk.testing.TestPipeline;
+import org.apache.beam.vendor.guava.v26_0_jre.com.google.common.collect.ImmutableSet;
+import org.hamcrest.Matchers;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public class SpannerChangeStreamErrorTest implements Serializable {
+
+  public static final String SPANNER_HOST = "my-host";
+  private static final String TEST_PROJECT = "my-project";
+  private static final String TEST_INSTANCE = "my-instance";
+  private static final String TEST_DATABASE = "my-database";
+  private static final String TEST_TABLE = "my-metadata-table";
+  private static final String TEST_CHANGE_STREAM = "my-change-stream";
+
+  @Rule
+  public final transient TestPipeline pipeline =
+      TestPipeline.create().enableAbandonedNodeEnforcement(false);
+
+  @Rule public final transient ExpectedException thrown = ExpectedException.none();
+
+  private MockSpannerServiceImpl mockSpannerService;
+  private MockServiceHelper serviceHelper;
+
+  @Before
+  public void setUp() throws Exception {
+    mockSpannerService = new MockSpannerServiceImpl();
+    serviceHelper =
+        new MockServiceHelper(SPANNER_HOST, Collections.singletonList(mockSpannerService));
+    serviceHelper.start();
+    serviceHelper.reset();
+  }
+
+  @After
+  public void tearDown() throws NoSuchFieldException, IllegalAccessException {
+    serviceHelper.reset();
+    serviceHelper.stop();
+    mockSpannerService.reset();
+    resetDaoFactoryFields();
+  }
+
+  @Test
+  public void testResourceExhaustedDoesNotRetry() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofStickyException(Status.RESOURCE_EXHAUSTED.asRuntimeException()));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.RESOURCE_EXHAUSTED.name());
+    }
+  }
+
+  @Test
+  public void testUnavailableExceptionRetries() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofExceptions(
+            ImmutableSet.of(
+                Status.UNAVAILABLE.asRuntimeException(),
+                Status.RESOURCE_EXHAUSTED.asRuntimeException())));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      assertThat(
+          mockSpannerService.countRequestsOfType(ExecuteSqlRequest.class), Matchers.greaterThan(1));
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.RESOURCE_EXHAUSTED.name());
+    }
+  }
+
+  @Test
+  public void testAbortedExceptionRetries() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofStickyException(Status.ABORTED.asRuntimeException()));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      assertThat(
+          mockSpannerService.countRequestsOfType(ExecuteSqlRequest.class), Matchers.greaterThan(1));
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.ABORTED.name());
+    }
+  }
+
+  @Test
+  public void testUnknownExceptionDoesNotRetry() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofStickyException(Status.UNKNOWN.asRuntimeException()));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      assertThat(
+          mockSpannerService.countRequestsOfType(ExecuteSqlRequest.class), Matchers.equalTo(1));
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.UNKNOWN.name());
+    }
+  }
+
+  @Test
+  public void testInvalidRecordReceived() {
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+
+    mockTableExists();
+    ResultSet getPartitionResultSet = mockGetParentPartition(now, after3Seconds);
+    mockGetWatermark(now);
+    mockGetPartitionsAfter(
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds(), now.getNanos() + 1000),
+        getPartitionResultSet);
+    mockGetPartitionsAfter(
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds(), now.getNanos() - 1000),
+        getPartitionResultSet);
+
+    Statement changeStreamQueryStatement =
+        Statement.newBuilder(
+                "SELECT * FROM READ_my-change-stream(   start_timestamp => @startTimestamp,   end_timestamp => @endTimestamp,   partition_token => @partitionToken,   read_options => null,   heartbeat_milliseconds => @heartbeatMillis)")
+            .bind("startTimestamp")
+            .to(now)
+            .bind("endTimestamp")
+            .to(after3Seconds)
+            .bind("partitionToken")
+            .to((String) null)
+            .bind("heartbeatMillis")
+            .to(500)
+            .build();
+    ResultSetMetadata readChangeStreamResultSetMetadata =

Review comment:
       nit: could we extract these to methods as well?




-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] zoercai commented on a change in pull request #16846: [BEAM-12164]: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
zoercai commented on a change in pull request #16846:
URL: https://github.com/apache/beam/pull/16846#discussion_r808594540



##########
File path: buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy
##########
@@ -454,8 +454,12 @@ class BeamModulePlugin implements Plugin<Project> {
     def checkerframework_version = "3.10.0"
     def classgraph_version = "4.8.104"
     def errorprone_version = "2.10.0"
+    // Try to keep gax_version consistent with gax-grpc version in google_cloud_platform_libraries_bom
+    def gax_version = "2.8.1"

Review comment:
       I did try this without the version, but the URL becomes invalid if the version is not between the `gax-grpc:` and the `:testlib` on line 549. This is the error produced without the version:
   
   ![image](https://user-images.githubusercontent.com/8233863/154387064-7ecad690-c61f-40f2-bfd8-0c62edeb4d5c.png)
   
   Let me know if you know of another way!
   




-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] codecov[bot] edited a comment on pull request #16846: [BEAM-12164]: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16846:
URL: https://github.com/apache/beam/pull/16846#issuecomment-1039895718






-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] codecov[bot] edited a comment on pull request #16846: [BEAM-12164]: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16846:
URL: https://github.com/apache/beam/pull/16846#issuecomment-1039895718


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#16846](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (16ce8d8) into [master](https://codecov.io/gh/apache/beam/commit/8bf68ad5023c30cd56ec63d2084017b4a547fe48?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8bf68ad) will **increase** coverage by `8.95%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16846/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #16846      +/-   ##
   ==========================================
   + Coverage   74.63%   83.59%   +8.95%     
   ==========================================
     Files         655      452     -203     
     Lines       82237    62291   -19946     
   ==========================================
   - Hits        61380    52073    -9307     
   + Misses      19861    10218    -9643     
   + Partials      996        0     -996     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/utils/interactive\_utils.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvaW50ZXJhY3RpdmVfdXRpbHMucHk=) | `87.80% <0.00%> (-7.32%)` | :arrow_down: |
   | [sdks/python/apache\_beam/io/jdbc.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vamRiYy5weQ==) | `78.26% <0.00%> (-6.74%)` | :arrow_down: |
   | [...s/python/apache\_beam/runners/portability/stager.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9zdGFnZXIucHk=) | `85.54% <0.00%> (-2.82%)` | :arrow_down: |
   | [.../python/apache\_beam/transforms/periodicsequence.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9wZXJpb2RpY3NlcXVlbmNlLnB5) | `96.72% <0.00%> (-1.64%)` | :arrow_down: |
   | [...pache\_beam/runners/interactive/interactive\_beam.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9iZWFtLnB5) | `76.41% <0.00%> (-0.95%)` | :arrow_down: |
   | [...runners/interactive/display/pcoll\_visualization.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9kaXNwbGF5L3Bjb2xsX3Zpc3VhbGl6YXRpb24ucHk=) | `85.85% <0.00%> (-0.51%)` | :arrow_down: |
   | [...eam/runners/interactive/interactive\_environment.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9lbnZpcm9ubWVudC5weQ==) | `90.09% <0.00%> (-0.31%)` | :arrow_down: |
   | [setup.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2V0dXAucHk=) | `0.00% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/transforms/core.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9jb3JlLnB5) | `92.64% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/portability/common\_urns.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvY29tbW9uX3VybnMucHk=) | `100.00% <0.00%> (ø)` | |
   | ... and [217 more](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [8bf68ad...16ce8d8](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] codecov[bot] edited a comment on pull request #16846: [BEAM-12164]: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16846:
URL: https://github.com/apache/beam/pull/16846#issuecomment-1039895718


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#16846](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (16ce8d8) into [master](https://codecov.io/gh/apache/beam/commit/8bf68ad5023c30cd56ec63d2084017b4a547fe48?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8bf68ad) will **increase** coverage by `8.95%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16846/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #16846      +/-   ##
   ==========================================
   + Coverage   74.63%   83.59%   +8.95%     
   ==========================================
     Files         655      452     -203     
     Lines       82237    62291   -19946     
   ==========================================
   - Hits        61380    52073    -9307     
   + Misses      19861    10218    -9643     
   + Partials      996        0     -996     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/utils/interactive\_utils.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvaW50ZXJhY3RpdmVfdXRpbHMucHk=) | `87.80% <0.00%> (-7.32%)` | :arrow_down: |
   | [sdks/python/apache\_beam/io/jdbc.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vamRiYy5weQ==) | `78.26% <0.00%> (-6.74%)` | :arrow_down: |
   | [...s/python/apache\_beam/runners/portability/stager.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9zdGFnZXIucHk=) | `85.54% <0.00%> (-2.82%)` | :arrow_down: |
   | [.../python/apache\_beam/transforms/periodicsequence.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9wZXJpb2RpY3NlcXVlbmNlLnB5) | `96.72% <0.00%> (-1.64%)` | :arrow_down: |
   | [...pache\_beam/runners/interactive/interactive\_beam.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9iZWFtLnB5) | `76.41% <0.00%> (-0.95%)` | :arrow_down: |
   | [...runners/interactive/display/pcoll\_visualization.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9kaXNwbGF5L3Bjb2xsX3Zpc3VhbGl6YXRpb24ucHk=) | `85.85% <0.00%> (-0.51%)` | :arrow_down: |
   | [...eam/runners/interactive/interactive\_environment.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9lbnZpcm9ubWVudC5weQ==) | `90.09% <0.00%> (-0.31%)` | :arrow_down: |
   | [setup.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2V0dXAucHk=) | `0.00% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/transforms/core.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9jb3JlLnB5) | `92.64% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/portability/common\_urns.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvY29tbW9uX3VybnMucHk=) | `100.00% <0.00%> (ø)` | |
   | ... and [217 more](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [8bf68ad...16ce8d8](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] zoercai commented on pull request #16846: [BEAM-12164]: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
zoercai commented on pull request #16846:
URL: https://github.com/apache/beam/pull/16846#issuecomment-1042539509


   @thiagotnunes I've made a PR (https://github.com/apache/beam/pull/16879) with the javadocs which I'll rebase once this PR is merged.


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] codecov[bot] commented on pull request #16846: [BEAM-12164]: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #16846:
URL: https://github.com/apache/beam/pull/16846#issuecomment-1039895718


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#16846](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (16ce8d8) into [master](https://codecov.io/gh/apache/beam/commit/8bf68ad5023c30cd56ec63d2084017b4a547fe48?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8bf68ad) will **increase** coverage by `8.95%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16846/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #16846      +/-   ##
   ==========================================
   + Coverage   74.63%   83.59%   +8.95%     
   ==========================================
     Files         655      452     -203     
     Lines       82237    62291   -19946     
   ==========================================
   - Hits        61380    52073    -9307     
   + Misses      19861    10218    -9643     
   + Partials      996        0     -996     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/utils/interactive\_utils.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvaW50ZXJhY3RpdmVfdXRpbHMucHk=) | `87.80% <0.00%> (-7.32%)` | :arrow_down: |
   | [sdks/python/apache\_beam/io/jdbc.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vamRiYy5weQ==) | `78.26% <0.00%> (-6.74%)` | :arrow_down: |
   | [...s/python/apache\_beam/runners/portability/stager.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9zdGFnZXIucHk=) | `85.54% <0.00%> (-2.82%)` | :arrow_down: |
   | [.../python/apache\_beam/transforms/periodicsequence.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9wZXJpb2RpY3NlcXVlbmNlLnB5) | `96.72% <0.00%> (-1.64%)` | :arrow_down: |
   | [...pache\_beam/runners/interactive/interactive\_beam.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9iZWFtLnB5) | `76.41% <0.00%> (-0.95%)` | :arrow_down: |
   | [...runners/interactive/display/pcoll\_visualization.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9kaXNwbGF5L3Bjb2xsX3Zpc3VhbGl6YXRpb24ucHk=) | `85.85% <0.00%> (-0.51%)` | :arrow_down: |
   | [...eam/runners/interactive/interactive\_environment.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9lbnZpcm9ubWVudC5weQ==) | `90.09% <0.00%> (-0.31%)` | :arrow_down: |
   | [setup.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2V0dXAucHk=) | `0.00% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/transforms/core.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9jb3JlLnB5) | `92.64% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/portability/common\_urns.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvY29tbW9uX3VybnMucHk=) | `100.00% <0.00%> (ø)` | |
   | ... and [217 more](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [8bf68ad...16ce8d8](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] codecov[bot] edited a comment on pull request #16846: [BEAM-12164]: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16846:
URL: https://github.com/apache/beam/pull/16846#issuecomment-1039895718


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#16846](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0e12e60) into [master](https://codecov.io/gh/apache/beam/commit/8bf68ad5023c30cd56ec63d2084017b4a547fe48?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8bf68ad) will **increase** coverage by `8.99%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16846/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #16846      +/-   ##
   ==========================================
   + Coverage   74.63%   83.63%   +8.99%     
   ==========================================
     Files         655      453     -202     
     Lines       82237    62471   -19766     
   ==========================================
   - Hits        61380    52245    -9135     
   + Misses      19861    10226    -9635     
   + Partials      996        0     -996     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/io/jdbc.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vamRiYy5weQ==) | `78.26% <0.00%> (-6.74%)` | :arrow_down: |
   | [...s/python/apache\_beam/runners/portability/stager.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9zdGFnZXIucHk=) | `85.54% <0.00%> (-2.82%)` | :arrow_down: |
   | [...apache\_beam/runners/portability/portable\_runner.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9wb3J0YWJsZV9ydW5uZXIucHk=) | `75.87% <0.00%> (-0.27%)` | :arrow_down: |
   | [setup.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2V0dXAucHk=) | `0.00% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/transforms/core.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9jb3JlLnB5) | `92.64% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/portability/common\_urns.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvY29tbW9uX3VybnMucHk=) | `100.00% <0.00%> (ø)` | |
   | [...python/apache\_beam/runners/direct/direct\_runner.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kaXJlY3QvZGlyZWN0X3J1bm5lci5weQ==) | `93.92% <0.00%> (ø)` | |
   | [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.39% <0.00%> (ø)` | |
   | [...on/apache\_beam/examples/dataframe/flight\_delays.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZXhhbXBsZXMvZGF0YWZyYW1lL2ZsaWdodF9kZWxheXMucHk=) | `27.90% <0.00%> (ø)` | |
   | [...apache\_beam/portability/api/beam\_runner\_api\_pb2.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvYXBpL2JlYW1fcnVubmVyX2FwaV9wYjIucHk=) | `100.00% <0.00%> (ø)` | |
   | ... and [220 more](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [8bf68ad...0e12e60](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] codecov[bot] edited a comment on pull request #16846: [BEAM-12164]: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16846:
URL: https://github.com/apache/beam/pull/16846#issuecomment-1039895718


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#16846](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0e12e60) into [master](https://codecov.io/gh/apache/beam/commit/8bf68ad5023c30cd56ec63d2084017b4a547fe48?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8bf68ad) will **increase** coverage by `8.99%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16846/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #16846      +/-   ##
   ==========================================
   + Coverage   74.63%   83.63%   +8.99%     
   ==========================================
     Files         655      453     -202     
     Lines       82237    62471   -19766     
   ==========================================
   - Hits        61380    52245    -9135     
   + Misses      19861    10226    -9635     
   + Partials      996        0     -996     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/io/jdbc.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vamRiYy5weQ==) | `78.26% <0.00%> (-6.74%)` | :arrow_down: |
   | [...s/python/apache\_beam/runners/portability/stager.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9zdGFnZXIucHk=) | `85.54% <0.00%> (-2.82%)` | :arrow_down: |
   | [...apache\_beam/runners/portability/portable\_runner.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9wb3J0YWJsZV9ydW5uZXIucHk=) | `75.87% <0.00%> (-0.27%)` | :arrow_down: |
   | [setup.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2V0dXAucHk=) | `0.00% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/transforms/core.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9jb3JlLnB5) | `92.64% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/portability/common\_urns.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvY29tbW9uX3VybnMucHk=) | `100.00% <0.00%> (ø)` | |
   | [...python/apache\_beam/runners/direct/direct\_runner.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kaXJlY3QvZGlyZWN0X3J1bm5lci5weQ==) | `93.92% <0.00%> (ø)` | |
   | [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.39% <0.00%> (ø)` | |
   | [...on/apache\_beam/examples/dataframe/flight\_delays.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZXhhbXBsZXMvZGF0YWZyYW1lL2ZsaWdodF9kZWxheXMucHk=) | `27.90% <0.00%> (ø)` | |
   | [...apache\_beam/portability/api/beam\_runner\_api\_pb2.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvYXBpL2JlYW1fcnVubmVyX2FwaV9wYjIucHk=) | `100.00% <0.00%> (ø)` | |
   | ... and [220 more](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [8bf68ad...0e12e60](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] codecov[bot] edited a comment on pull request #16846: [BEAM-12164]: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16846:
URL: https://github.com/apache/beam/pull/16846#issuecomment-1039895718


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#16846](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (16ce8d8) into [master](https://codecov.io/gh/apache/beam/commit/8bf68ad5023c30cd56ec63d2084017b4a547fe48?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8bf68ad) will **increase** coverage by `8.95%`.
   > The diff coverage is `n/a`.
   
   > :exclamation: Current head 16ce8d8 differs from pull request most recent head 361e6da. Consider uploading reports for the commit 361e6da to get more accurate results
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16846/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #16846      +/-   ##
   ==========================================
   + Coverage   74.63%   83.59%   +8.95%     
   ==========================================
     Files         655      452     -203     
     Lines       82237    62291   -19946     
   ==========================================
   - Hits        61380    52073    -9307     
   + Misses      19861    10218    -9643     
   + Partials      996        0     -996     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/utils/interactive\_utils.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvaW50ZXJhY3RpdmVfdXRpbHMucHk=) | `87.80% <0.00%> (-7.32%)` | :arrow_down: |
   | [sdks/python/apache\_beam/io/jdbc.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vamRiYy5weQ==) | `78.26% <0.00%> (-6.74%)` | :arrow_down: |
   | [...s/python/apache\_beam/runners/portability/stager.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9zdGFnZXIucHk=) | `85.54% <0.00%> (-2.82%)` | :arrow_down: |
   | [.../python/apache\_beam/transforms/periodicsequence.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9wZXJpb2RpY3NlcXVlbmNlLnB5) | `96.72% <0.00%> (-1.64%)` | :arrow_down: |
   | [...pache\_beam/runners/interactive/interactive\_beam.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9iZWFtLnB5) | `76.41% <0.00%> (-0.95%)` | :arrow_down: |
   | [...runners/interactive/display/pcoll\_visualization.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9kaXNwbGF5L3Bjb2xsX3Zpc3VhbGl6YXRpb24ucHk=) | `85.85% <0.00%> (-0.51%)` | :arrow_down: |
   | [...eam/runners/interactive/interactive\_environment.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9lbnZpcm9ubWVudC5weQ==) | `90.09% <0.00%> (-0.31%)` | :arrow_down: |
   | [setup.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2V0dXAucHk=) | `0.00% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/transforms/core.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9jb3JlLnB5) | `92.64% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/portability/common\_urns.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvY29tbW9uX3VybnMucHk=) | `100.00% <0.00%> (ø)` | |
   | ... and [217 more](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [8bf68ad...361e6da](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] codecov[bot] edited a comment on pull request #16846: [BEAM-12164]: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16846:
URL: https://github.com/apache/beam/pull/16846#issuecomment-1039895718


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#16846](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (361e6da) into [master](https://codecov.io/gh/apache/beam/commit/8bf68ad5023c30cd56ec63d2084017b4a547fe48?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8bf68ad) will **increase** coverage by `8.97%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16846/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #16846      +/-   ##
   ==========================================
   + Coverage   74.63%   83.61%   +8.97%     
   ==========================================
     Files         655      452     -203     
     Lines       82237    62292   -19945     
   ==========================================
   - Hits        61380    52085    -9295     
   + Misses      19861    10207    -9654     
   + Partials      996        0     -996     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/io/jdbc.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vamRiYy5weQ==) | `78.26% <0.00%> (-6.74%)` | :arrow_down: |
   | [...s/python/apache\_beam/runners/portability/stager.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9zdGFnZXIucHk=) | `85.54% <0.00%> (-2.82%)` | :arrow_down: |
   | [setup.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2V0dXAucHk=) | `0.00% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/transforms/core.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9jb3JlLnB5) | `92.64% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/portability/common\_urns.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvY29tbW9uX3VybnMucHk=) | `100.00% <0.00%> (ø)` | |
   | [...python/apache\_beam/runners/direct/direct\_runner.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kaXJlY3QvZGlyZWN0X3J1bm5lci5weQ==) | `93.92% <0.00%> (ø)` | |
   | [...on/apache\_beam/examples/dataframe/flight\_delays.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZXhhbXBsZXMvZGF0YWZyYW1lL2ZsaWdodF9kZWxheXMucHk=) | `27.90% <0.00%> (ø)` | |
   | [...apache\_beam/portability/api/beam\_runner\_api\_pb2.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvYXBpL2JlYW1fcnVubmVyX2FwaV9wYjIucHk=) | `100.00% <0.00%> (ø)` | |
   | [...apache\_beam/typehints/native\_type\_compatibility.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHlwZWhpbnRzL25hdGl2ZV90eXBlX2NvbXBhdGliaWxpdHkucHk=) | `86.57% <0.00%> (ø)` | |
   | [...e\_beam/portability/api/beam\_runner\_api\_pb2\_urns.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvYXBpL2JlYW1fcnVubmVyX2FwaV9wYjJfdXJucy5weQ==) | `100.00% <0.00%> (ø)` | |
   | ... and [215 more](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [8bf68ad...361e6da](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] thiagotnunes commented on pull request #16846: test: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
thiagotnunes commented on pull request #16846:
URL: https://github.com/apache/beam/pull/16846#issuecomment-1039887585


   You should change the title to match the JIRA ticket `[BEAM-12164]: <title>`


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] thiagotnunes commented on a change in pull request #16846: test: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
thiagotnunes commented on a change in pull request #16846:
URL: https://github.com/apache/beam/pull/16846#discussion_r805613897



##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/dao/DaoFactory.java
##########
@@ -133,4 +133,11 @@ public synchronized ChangeStreamDao getChangeStreamDao() {
     }
     return changeStreamDaoInstance;
   }
+
+  /** Resets the DaoFactory for testing. */
+  public static void reset() {

Review comment:
       This public method worries me a bit, because we will allow for users to mutate state of this instance. Other than that, it is not in a synchronized block, so it may break in a concurrent environment. Anyway we could inject a new instance of the DaoFactory on every test instead?

##########
File path: sdks/java/io/google-cloud-platform/build.gradle
##########
@@ -146,6 +147,7 @@ dependencies {
   testImplementation library.java.powermock
   testImplementation library.java.powermock_mockito
   testImplementation library.java.joda_time
+  testImplementation "com.google.cloud:google-cloud-spanner:6.17.4:tests"

Review comment:
       Can the version come from the BOM? (if you remove the version, does it work?)
   
   Could you create a variable as the other libraries (e.g. `library.java.spanner.tests`)?

##########
File path: sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/SpannerChangeStreamErrorTest.java
##########
@@ -0,0 +1,481 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.spanner.changestreams;
+
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_CREATED_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_END_TIMESTAMP;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_FINISHED_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_HEARTBEAT_MILLIS;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_PARENT_TOKENS;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_PARTITION_TOKEN;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_RUNNING_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_SCHEDULED_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_START_TIMESTAMP;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_STATE;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_WATERMARK;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.google.api.gax.grpc.testing.MockServiceHelper;
+import com.google.api.gax.retrying.RetrySettings;
+import com.google.cloud.Timestamp;
+import com.google.cloud.spanner.ErrorCode;
+import com.google.cloud.spanner.MockSpannerServiceImpl;
+import com.google.cloud.spanner.MockSpannerServiceImpl.SimulatedExecutionTime;
+import com.google.cloud.spanner.MockSpannerServiceImpl.StatementResult;
+import com.google.cloud.spanner.Statement;
+import com.google.protobuf.ListValue;
+import com.google.protobuf.NullValue;
+import com.google.protobuf.Value;
+import com.google.spanner.v1.ExecuteSqlRequest;
+import com.google.spanner.v1.ResultSet;
+import com.google.spanner.v1.ResultSetMetadata;
+import com.google.spanner.v1.StructType;
+import com.google.spanner.v1.StructType.Field;
+import com.google.spanner.v1.Type;
+import com.google.spanner.v1.TypeCode;
+import io.grpc.Status;
+import java.io.Serializable;
+import java.util.Collections;
+import org.apache.beam.sdk.Pipeline.PipelineExecutionException;
+import org.apache.beam.sdk.io.gcp.spanner.SpannerConfig;
+import org.apache.beam.sdk.io.gcp.spanner.SpannerIO;
+import org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.DaoFactory;
+import org.apache.beam.sdk.io.gcp.spanner.changestreams.model.PartitionMetadata.State;
+import org.apache.beam.sdk.options.ValueProvider.StaticValueProvider;
+import org.apache.beam.sdk.testing.TestPipeline;
+import org.apache.beam.vendor.guava.v26_0_jre.com.google.common.collect.ImmutableSet;
+import org.hamcrest.Matchers;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public class SpannerChangeStreamErrorTest implements Serializable {
+
+  public static final String SPANNER_HOST = "my-host";
+  private static final String TEST_PROJECT = "my-project";
+  private static final String TEST_INSTANCE = "my-instance";
+  private static final String TEST_DATABASE = "my-database";
+  private static final String TEST_TABLE = "my-metadata-table";
+  private static final String TEST_CHANGE_STREAM = "my-change-stream";
+
+  @Rule
+  public final transient TestPipeline pipeline =
+      TestPipeline.create().enableAbandonedNodeEnforcement(false);
+
+  @Rule public final transient ExpectedException thrown = ExpectedException.none();
+
+  private MockSpannerServiceImpl mockSpannerService;
+  private MockServiceHelper serviceHelper;
+
+  @Before
+  public void setUp() throws Exception {
+    mockSpannerService = new MockSpannerServiceImpl();
+    serviceHelper =
+        new MockServiceHelper(SPANNER_HOST, Collections.singletonList(mockSpannerService));
+    serviceHelper.start();
+    serviceHelper.reset();
+  }
+
+  @After
+  public void tearDown() {
+    serviceHelper.reset();
+    serviceHelper.stop();
+    mockSpannerService.reset();
+    DaoFactory.reset();
+  }
+
+  @Test
+  public void testResourceExhaustedDoesNotRetry() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofStickyException(Status.RESOURCE_EXHAUSTED.asRuntimeException()));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.RESOURCE_EXHAUSTED.name());
+    }
+  }
+
+  @Test
+  public void testUnavailableExceptionRetries() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofExceptions(
+            ImmutableSet.of(
+                Status.UNAVAILABLE.asRuntimeException(),
+                Status.RESOURCE_EXHAUSTED.asRuntimeException())));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      assertThat(
+          mockSpannerService.countRequestsOfType(ExecuteSqlRequest.class), Matchers.greaterThan(1));
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.RESOURCE_EXHAUSTED.name());
+    }
+  }
+
+  @Test
+  public void testAbortedExceptionRetries() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofStickyException(Status.ABORTED.asRuntimeException()));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      assertThat(
+          mockSpannerService.countRequestsOfType(ExecuteSqlRequest.class), Matchers.greaterThan(1));
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.ABORTED.name());
+    }
+  }
+
+  @Test
+  public void testUnknownExceptionDoesNotRetry() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofStickyException(Status.UNKNOWN.asRuntimeException()));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      assertThat(
+          mockSpannerService.countRequestsOfType(ExecuteSqlRequest.class), Matchers.equalTo(1));
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.UNKNOWN.name());
+    }
+  }
+
+  @Test
+  public void testInvalidRecordReceived() {
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+
+    Statement tableExistsStatement =
+        Statement.of(
+            "SELECT t.table_name FROM information_schema.tables AS t WHERE t.table_catalog = '' AND t.table_schema = '' AND t.table_name = 'my-metadata-table'");
+    ResultSetMetadata tableExistsResultSetMetadata =
+        ResultSetMetadata.newBuilder()
+            .setRowType(
+                StructType.newBuilder()
+                    .addFields(
+                        Field.newBuilder()
+                            .setName("table_name")
+                            .setType(Type.newBuilder().setCode(TypeCode.STRING).build())
+                            .build())
+                    .build())
+            .build();
+    ResultSet tableExistsResultSet =
+        ResultSet.newBuilder()
+            .addRows(
+                ListValue.newBuilder()
+                    .addValues(Value.newBuilder().setStringValue(TEST_TABLE).build())
+                    .build())
+            .setMetadata(tableExistsResultSetMetadata)
+            .build();
+    mockSpannerService.putStatementResult(
+        StatementResult.query(tableExistsStatement, tableExistsResultSet));
+
+    Statement getPartitionStatement =
+        Statement.newBuilder("SELECT * FROM my-metadata-table WHERE PartitionToken = @partition")
+            .bind("partition")
+            .to("Parent0")
+            .build();
+    ResultSetMetadata partitionMetadataResultSetMetadata =

Review comment:
       nit: this could be static outside of the test to make the test smaller.

##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java
##########
@@ -107,34 +108,56 @@ public static SpannerAccessor getOrCreate(SpannerConfig spannerConfig) {
   private static SpannerAccessor createAndConnect(SpannerConfig spannerConfig) {
     SpannerOptions.Builder builder = SpannerOptions.newBuilder();
 
-    ValueProvider<Duration> commitDeadline = spannerConfig.getCommitDeadline();
-    if (commitDeadline != null && commitDeadline.get().getMillis() > 0) {
+    // Set retryable codes
+    if (spannerConfig.getRetryableCodes() != null) {

Review comment:
       I am not sure how to model this yet, but if we do it by creating a method for each setting, it might become too large, so I was thinking that maybe we could pass in a `SpannerConfig.setSpannerStubSettings()` and extract all the settings from that.

##########
File path: sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/SpannerChangeStreamErrorTest.java
##########
@@ -0,0 +1,481 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.spanner.changestreams;
+
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_CREATED_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_END_TIMESTAMP;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_FINISHED_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_HEARTBEAT_MILLIS;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_PARENT_TOKENS;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_PARTITION_TOKEN;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_RUNNING_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_SCHEDULED_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_START_TIMESTAMP;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_STATE;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_WATERMARK;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.google.api.gax.grpc.testing.MockServiceHelper;
+import com.google.api.gax.retrying.RetrySettings;
+import com.google.cloud.Timestamp;
+import com.google.cloud.spanner.ErrorCode;
+import com.google.cloud.spanner.MockSpannerServiceImpl;
+import com.google.cloud.spanner.MockSpannerServiceImpl.SimulatedExecutionTime;
+import com.google.cloud.spanner.MockSpannerServiceImpl.StatementResult;
+import com.google.cloud.spanner.Statement;
+import com.google.protobuf.ListValue;
+import com.google.protobuf.NullValue;
+import com.google.protobuf.Value;
+import com.google.spanner.v1.ExecuteSqlRequest;
+import com.google.spanner.v1.ResultSet;
+import com.google.spanner.v1.ResultSetMetadata;
+import com.google.spanner.v1.StructType;
+import com.google.spanner.v1.StructType.Field;
+import com.google.spanner.v1.Type;
+import com.google.spanner.v1.TypeCode;
+import io.grpc.Status;
+import java.io.Serializable;
+import java.util.Collections;
+import org.apache.beam.sdk.Pipeline.PipelineExecutionException;
+import org.apache.beam.sdk.io.gcp.spanner.SpannerConfig;
+import org.apache.beam.sdk.io.gcp.spanner.SpannerIO;
+import org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.DaoFactory;
+import org.apache.beam.sdk.io.gcp.spanner.changestreams.model.PartitionMetadata.State;
+import org.apache.beam.sdk.options.ValueProvider.StaticValueProvider;
+import org.apache.beam.sdk.testing.TestPipeline;
+import org.apache.beam.vendor.guava.v26_0_jre.com.google.common.collect.ImmutableSet;
+import org.hamcrest.Matchers;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public class SpannerChangeStreamErrorTest implements Serializable {
+
+  public static final String SPANNER_HOST = "my-host";
+  private static final String TEST_PROJECT = "my-project";
+  private static final String TEST_INSTANCE = "my-instance";
+  private static final String TEST_DATABASE = "my-database";
+  private static final String TEST_TABLE = "my-metadata-table";
+  private static final String TEST_CHANGE_STREAM = "my-change-stream";
+
+  @Rule
+  public final transient TestPipeline pipeline =
+      TestPipeline.create().enableAbandonedNodeEnforcement(false);
+
+  @Rule public final transient ExpectedException thrown = ExpectedException.none();
+
+  private MockSpannerServiceImpl mockSpannerService;
+  private MockServiceHelper serviceHelper;
+
+  @Before
+  public void setUp() throws Exception {
+    mockSpannerService = new MockSpannerServiceImpl();
+    serviceHelper =
+        new MockServiceHelper(SPANNER_HOST, Collections.singletonList(mockSpannerService));
+    serviceHelper.start();
+    serviceHelper.reset();
+  }
+
+  @After
+  public void tearDown() {
+    serviceHelper.reset();
+    serviceHelper.stop();
+    mockSpannerService.reset();
+    DaoFactory.reset();
+  }
+
+  @Test
+  public void testResourceExhaustedDoesNotRetry() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofStickyException(Status.RESOURCE_EXHAUSTED.asRuntimeException()));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.RESOURCE_EXHAUSTED.name());
+    }
+  }
+
+  @Test
+  public void testUnavailableExceptionRetries() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofExceptions(
+            ImmutableSet.of(
+                Status.UNAVAILABLE.asRuntimeException(),
+                Status.RESOURCE_EXHAUSTED.asRuntimeException())));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      assertThat(
+          mockSpannerService.countRequestsOfType(ExecuteSqlRequest.class), Matchers.greaterThan(1));
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.RESOURCE_EXHAUSTED.name());
+    }
+  }
+
+  @Test
+  public void testAbortedExceptionRetries() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofStickyException(Status.ABORTED.asRuntimeException()));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      assertThat(
+          mockSpannerService.countRequestsOfType(ExecuteSqlRequest.class), Matchers.greaterThan(1));
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.ABORTED.name());
+    }
+  }
+
+  @Test
+  public void testUnknownExceptionDoesNotRetry() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofStickyException(Status.UNKNOWN.asRuntimeException()));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      assertThat(
+          mockSpannerService.countRequestsOfType(ExecuteSqlRequest.class), Matchers.equalTo(1));
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.UNKNOWN.name());
+    }
+  }
+
+  @Test
+  public void testInvalidRecordReceived() {
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+
+    Statement tableExistsStatement =
+        Statement.of(
+            "SELECT t.table_name FROM information_schema.tables AS t WHERE t.table_catalog = '' AND t.table_schema = '' AND t.table_name = 'my-metadata-table'");
+    ResultSetMetadata tableExistsResultSetMetadata =
+        ResultSetMetadata.newBuilder()
+            .setRowType(
+                StructType.newBuilder()
+                    .addFields(
+                        Field.newBuilder()
+                            .setName("table_name")
+                            .setType(Type.newBuilder().setCode(TypeCode.STRING).build())
+                            .build())
+                    .build())
+            .build();
+    ResultSet tableExistsResultSet =
+        ResultSet.newBuilder()
+            .addRows(
+                ListValue.newBuilder()
+                    .addValues(Value.newBuilder().setStringValue(TEST_TABLE).build())
+                    .build())
+            .setMetadata(tableExistsResultSetMetadata)
+            .build();
+    mockSpannerService.putStatementResult(
+        StatementResult.query(tableExistsStatement, tableExistsResultSet));
+
+    Statement getPartitionStatement =
+        Statement.newBuilder("SELECT * FROM my-metadata-table WHERE PartitionToken = @partition")
+            .bind("partition")
+            .to("Parent0")
+            .build();
+    ResultSetMetadata partitionMetadataResultSetMetadata =
+        ResultSetMetadata.newBuilder()
+            .setRowType(
+                StructType.newBuilder()
+                    .addFields(
+                        Field.newBuilder()
+                            .setName(COLUMN_PARTITION_TOKEN)
+                            .setType(Type.newBuilder().setCode(TypeCode.STRING))
+                            .build())
+                    .addFields(
+                        Field.newBuilder()
+                            .setName(COLUMN_PARENT_TOKENS)
+                            .setType(
+                                Type.newBuilder()
+                                    .setCode(TypeCode.ARRAY)
+                                    .setArrayElementType(
+                                        Type.newBuilder().setCode(TypeCode.STRING)))
+                            .build())
+                    .addFields(
+                        Field.newBuilder()
+                            .setName(COLUMN_START_TIMESTAMP)
+                            .setType(Type.newBuilder().setCode(TypeCode.TIMESTAMP)))
+                    .addFields(
+                        Field.newBuilder()
+                            .setName(COLUMN_END_TIMESTAMP)
+                            .setType(Type.newBuilder().setCode(TypeCode.TIMESTAMP)))
+                    .addFields(
+                        Field.newBuilder()
+                            .setName(COLUMN_HEARTBEAT_MILLIS)
+                            .setType(Type.newBuilder().setCode(TypeCode.INT64)))
+                    .addFields(
+                        Field.newBuilder()
+                            .setName(COLUMN_STATE)
+                            .setType(Type.newBuilder().setCode(TypeCode.STRING)))
+                    .addFields(
+                        Field.newBuilder()
+                            .setName(COLUMN_WATERMARK)
+                            .setType(Type.newBuilder().setCode(TypeCode.TIMESTAMP)))
+                    .addFields(
+                        Field.newBuilder()
+                            .setName(COLUMN_CREATED_AT)
+                            .setType(Type.newBuilder().setCode(TypeCode.TIMESTAMP)))
+                    .addFields(
+                        Field.newBuilder()
+                            .setName(COLUMN_SCHEDULED_AT)
+                            .setType(Type.newBuilder().setCode(TypeCode.TIMESTAMP)))
+                    .addFields(
+                        Field.newBuilder()
+                            .setName(COLUMN_RUNNING_AT)
+                            .setType(Type.newBuilder().setCode(TypeCode.TIMESTAMP)))
+                    .addFields(
+                        Field.newBuilder()
+                            .setName(COLUMN_FINISHED_AT)
+                            .setType(Type.newBuilder().setCode(TypeCode.TIMESTAMP)))
+                    .build())
+            .build();
+    ResultSet getPartitionResultSet =

Review comment:
       nit: Maybe we can create a method to hide this manual creation of the resultSet / metadata so that the test case is smaller?

##########
File path: sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/SpannerChangeStreamErrorTest.java
##########
@@ -0,0 +1,481 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.spanner.changestreams;
+
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_CREATED_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_END_TIMESTAMP;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_FINISHED_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_HEARTBEAT_MILLIS;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_PARENT_TOKENS;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_PARTITION_TOKEN;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_RUNNING_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_SCHEDULED_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_START_TIMESTAMP;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_STATE;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_WATERMARK;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.google.api.gax.grpc.testing.MockServiceHelper;
+import com.google.api.gax.retrying.RetrySettings;
+import com.google.cloud.Timestamp;
+import com.google.cloud.spanner.ErrorCode;
+import com.google.cloud.spanner.MockSpannerServiceImpl;
+import com.google.cloud.spanner.MockSpannerServiceImpl.SimulatedExecutionTime;
+import com.google.cloud.spanner.MockSpannerServiceImpl.StatementResult;
+import com.google.cloud.spanner.Statement;
+import com.google.protobuf.ListValue;
+import com.google.protobuf.NullValue;
+import com.google.protobuf.Value;
+import com.google.spanner.v1.ExecuteSqlRequest;
+import com.google.spanner.v1.ResultSet;
+import com.google.spanner.v1.ResultSetMetadata;
+import com.google.spanner.v1.StructType;
+import com.google.spanner.v1.StructType.Field;
+import com.google.spanner.v1.Type;
+import com.google.spanner.v1.TypeCode;
+import io.grpc.Status;
+import java.io.Serializable;
+import java.util.Collections;
+import org.apache.beam.sdk.Pipeline.PipelineExecutionException;
+import org.apache.beam.sdk.io.gcp.spanner.SpannerConfig;
+import org.apache.beam.sdk.io.gcp.spanner.SpannerIO;
+import org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.DaoFactory;
+import org.apache.beam.sdk.io.gcp.spanner.changestreams.model.PartitionMetadata.State;
+import org.apache.beam.sdk.options.ValueProvider.StaticValueProvider;
+import org.apache.beam.sdk.testing.TestPipeline;
+import org.apache.beam.vendor.guava.v26_0_jre.com.google.common.collect.ImmutableSet;
+import org.hamcrest.Matchers;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public class SpannerChangeStreamErrorTest implements Serializable {
+
+  public static final String SPANNER_HOST = "my-host";
+  private static final String TEST_PROJECT = "my-project";
+  private static final String TEST_INSTANCE = "my-instance";
+  private static final String TEST_DATABASE = "my-database";
+  private static final String TEST_TABLE = "my-metadata-table";
+  private static final String TEST_CHANGE_STREAM = "my-change-stream";
+
+  @Rule
+  public final transient TestPipeline pipeline =
+      TestPipeline.create().enableAbandonedNodeEnforcement(false);
+
+  @Rule public final transient ExpectedException thrown = ExpectedException.none();
+
+  private MockSpannerServiceImpl mockSpannerService;
+  private MockServiceHelper serviceHelper;
+
+  @Before
+  public void setUp() throws Exception {
+    mockSpannerService = new MockSpannerServiceImpl();
+    serviceHelper =
+        new MockServiceHelper(SPANNER_HOST, Collections.singletonList(mockSpannerService));
+    serviceHelper.start();
+    serviceHelper.reset();
+  }
+
+  @After
+  public void tearDown() {
+    serviceHelper.reset();
+    serviceHelper.stop();
+    mockSpannerService.reset();
+    DaoFactory.reset();
+  }
+
+  @Test
+  public void testResourceExhaustedDoesNotRetry() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofStickyException(Status.RESOURCE_EXHAUSTED.asRuntimeException()));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.RESOURCE_EXHAUSTED.name());
+    }
+  }
+
+  @Test
+  public void testUnavailableExceptionRetries() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofExceptions(
+            ImmutableSet.of(
+                Status.UNAVAILABLE.asRuntimeException(),
+                Status.RESOURCE_EXHAUSTED.asRuntimeException())));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      assertThat(
+          mockSpannerService.countRequestsOfType(ExecuteSqlRequest.class), Matchers.greaterThan(1));
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.RESOURCE_EXHAUSTED.name());
+    }
+  }
+
+  @Test
+  public void testAbortedExceptionRetries() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofStickyException(Status.ABORTED.asRuntimeException()));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      assertThat(
+          mockSpannerService.countRequestsOfType(ExecuteSqlRequest.class), Matchers.greaterThan(1));
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.ABORTED.name());
+    }
+  }
+
+  @Test
+  public void testUnknownExceptionDoesNotRetry() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofStickyException(Status.UNKNOWN.asRuntimeException()));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      assertThat(
+          mockSpannerService.countRequestsOfType(ExecuteSqlRequest.class), Matchers.equalTo(1));
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.UNKNOWN.name());
+    }
+  }
+
+  @Test
+  public void testInvalidRecordReceived() {
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+
+    Statement tableExistsStatement =
+        Statement.of(
+            "SELECT t.table_name FROM information_schema.tables AS t WHERE t.table_catalog = '' AND t.table_schema = '' AND t.table_name = 'my-metadata-table'");
+    ResultSetMetadata tableExistsResultSetMetadata =
+        ResultSetMetadata.newBuilder()
+            .setRowType(
+                StructType.newBuilder()
+                    .addFields(
+                        Field.newBuilder()
+                            .setName("table_name")
+                            .setType(Type.newBuilder().setCode(TypeCode.STRING).build())
+                            .build())
+                    .build())
+            .build();
+    ResultSet tableExistsResultSet =
+        ResultSet.newBuilder()
+            .addRows(
+                ListValue.newBuilder()
+                    .addValues(Value.newBuilder().setStringValue(TEST_TABLE).build())
+                    .build())
+            .setMetadata(tableExistsResultSetMetadata)
+            .build();
+    mockSpannerService.putStatementResult(
+        StatementResult.query(tableExistsStatement, tableExistsResultSet));
+
+    Statement getPartitionStatement =
+        Statement.newBuilder("SELECT * FROM my-metadata-table WHERE PartitionToken = @partition")
+            .bind("partition")
+            .to("Parent0")
+            .build();
+    ResultSetMetadata partitionMetadataResultSetMetadata =
+        ResultSetMetadata.newBuilder()
+            .setRowType(
+                StructType.newBuilder()
+                    .addFields(
+                        Field.newBuilder()
+                            .setName(COLUMN_PARTITION_TOKEN)
+                            .setType(Type.newBuilder().setCode(TypeCode.STRING))
+                            .build())
+                    .addFields(
+                        Field.newBuilder()
+                            .setName(COLUMN_PARENT_TOKENS)
+                            .setType(
+                                Type.newBuilder()
+                                    .setCode(TypeCode.ARRAY)
+                                    .setArrayElementType(
+                                        Type.newBuilder().setCode(TypeCode.STRING)))
+                            .build())
+                    .addFields(
+                        Field.newBuilder()
+                            .setName(COLUMN_START_TIMESTAMP)
+                            .setType(Type.newBuilder().setCode(TypeCode.TIMESTAMP)))
+                    .addFields(
+                        Field.newBuilder()
+                            .setName(COLUMN_END_TIMESTAMP)
+                            .setType(Type.newBuilder().setCode(TypeCode.TIMESTAMP)))
+                    .addFields(
+                        Field.newBuilder()
+                            .setName(COLUMN_HEARTBEAT_MILLIS)
+                            .setType(Type.newBuilder().setCode(TypeCode.INT64)))
+                    .addFields(
+                        Field.newBuilder()
+                            .setName(COLUMN_STATE)
+                            .setType(Type.newBuilder().setCode(TypeCode.STRING)))
+                    .addFields(
+                        Field.newBuilder()
+                            .setName(COLUMN_WATERMARK)
+                            .setType(Type.newBuilder().setCode(TypeCode.TIMESTAMP)))
+                    .addFields(
+                        Field.newBuilder()
+                            .setName(COLUMN_CREATED_AT)
+                            .setType(Type.newBuilder().setCode(TypeCode.TIMESTAMP)))
+                    .addFields(
+                        Field.newBuilder()
+                            .setName(COLUMN_SCHEDULED_AT)
+                            .setType(Type.newBuilder().setCode(TypeCode.TIMESTAMP)))
+                    .addFields(
+                        Field.newBuilder()
+                            .setName(COLUMN_RUNNING_AT)
+                            .setType(Type.newBuilder().setCode(TypeCode.TIMESTAMP)))
+                    .addFields(
+                        Field.newBuilder()
+                            .setName(COLUMN_FINISHED_AT)
+                            .setType(Type.newBuilder().setCode(TypeCode.TIMESTAMP)))
+                    .build())
+            .build();
+    ResultSet getPartitionResultSet =
+        ResultSet.newBuilder()
+            .addRows(
+                ListValue.newBuilder()
+                    .addValues(Value.newBuilder().setStringValue("Parent0"))
+                    .addValues(Value.newBuilder().setListValue(ListValue.newBuilder().build()))
+                    .addValues(Value.newBuilder().setStringValue(now.toString()))
+                    .addValues(Value.newBuilder().setStringValue(after3Seconds.toString()))
+                    .addValues(Value.newBuilder().setStringValue("500"))
+                    .addValues(Value.newBuilder().setStringValue(State.CREATED.name()))
+                    .addValues(Value.newBuilder().setStringValue(now.toString()))
+                    .addValues(Value.newBuilder().setStringValue(now.toString()))
+                    .addValues(Value.newBuilder().setNullValue(NullValue.NULL_VALUE).build())
+                    .addValues(Value.newBuilder().setNullValue(NullValue.NULL_VALUE).build())
+                    .addValues(Value.newBuilder().setNullValue(NullValue.NULL_VALUE).build())
+                    .build())
+            .setMetadata(partitionMetadataResultSetMetadata)
+            .build();
+    mockSpannerService.putStatementResult(
+        StatementResult.query(getPartitionStatement, getPartitionResultSet));
+
+    Statement watermarkStatement =
+        Statement.newBuilder(
+                "SELECT Watermark FROM my-metadata-table WHERE State != @state ORDER BY Watermark ASC LIMIT 1")
+            .bind("state")
+            .to(State.FINISHED.name())
+            .build();
+    ResultSetMetadata watermarkResultSetMetadata =
+        ResultSetMetadata.newBuilder()
+            .setRowType(
+                StructType.newBuilder()
+                    .addFields(
+                        Field.newBuilder()
+                            .setName("Watermark")
+                            .setType(Type.newBuilder().setCode(TypeCode.TIMESTAMP).build())
+                            .build())
+                    .build())
+            .build();
+    ResultSet watermarkResultSet =
+        ResultSet.newBuilder()
+            .addRows(
+                ListValue.newBuilder()
+                    .addValues(Value.newBuilder().setStringValue(now.toString()).build())
+                    .build())
+            .setMetadata(watermarkResultSetMetadata)
+            .build();
+    mockSpannerService.putStatementResult(
+        StatementResult.query(watermarkStatement, watermarkResultSet));
+
+    Statement getPartitionsAfterStatement =
+        Statement.newBuilder(
+                "SELECT * FROM my-metadata-table WHERE CreatedAt > @timestamp ORDER BY CreatedAt ASC, StartTimestamp ASC")
+            .bind("timestamp")
+            .to(Timestamp.ofTimeSecondsAndNanos(now.getSeconds(), now.getNanos() - 1_000))
+            .build();
+    mockSpannerService.putStatementResult(
+        StatementResult.query(getPartitionsAfterStatement, getPartitionResultSet));
+
+    Statement getPartitionsAfterStatement2 =
+        Statement.newBuilder(
+                "SELECT * FROM my-metadata-table WHERE CreatedAt > @timestamp ORDER BY CreatedAt ASC, StartTimestamp ASC")
+            .bind("timestamp")
+            .to(Timestamp.ofTimeSecondsAndNanos(now.getSeconds(), now.getNanos() + 1_000))
+            .build();
+    mockSpannerService.putStatementResult(
+        StatementResult.query(getPartitionsAfterStatement2, getPartitionResultSet));
+
+    Statement changeStreamQueryStatement =
+        Statement.newBuilder(
+                "SELECT * FROM READ_my-change-stream(   start_timestamp => @startTimestamp,   end_timestamp => @endTimestamp,   partition_token => @partitionToken,   read_options => null,   heartbeat_milliseconds => @heartbeatMillis)")
+            .bind("startTimestamp")
+            .to(now)
+            .bind("endTimestamp")
+            .to(after3Seconds)
+            .bind("partitionToken")
+            .to((String) null)
+            .bind("heartbeatMillis")
+            .to(500)
+            .build();
+    ResultSetMetadata readChangeStreamResultSetMetadata =
+        ResultSetMetadata.newBuilder()
+            .setRowType(
+                StructType.newBuilder()
+                    .addFields(
+                        Field.newBuilder()
+                            .setName("COL1")
+                            .setType(
+                                Type.newBuilder()
+                                    .setCode(TypeCode.ARRAY)
+                                    .setArrayElementType(
+                                        Type.newBuilder()
+                                            .setCode(TypeCode.STRUCT)
+                                            .setStructType(
+                                                StructType.newBuilder()
+                                                    .addFields(
+                                                        Field.newBuilder()
+                                                            .setName("field_name")
+                                                            .setType(
+                                                                Type.newBuilder()
+                                                                    .setCode(TypeCode.STRUCT)
+                                                                    .setStructType(
+                                                                        StructType.newBuilder()
+                                                                            .addFields(
+                                                                                Field.newBuilder()
+                                                                                    .setType(
+                                                                                        Type
+                                                                                            .newBuilder()
+                                                                                            .setCode(
+                                                                                                TypeCode
+                                                                                                    .STRING))
+                                                                                    .build())

Review comment:
       nit: I think you can remove the builds (but the most outer one) and it should still compile.

##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java
##########
@@ -107,34 +108,56 @@ public static SpannerAccessor getOrCreate(SpannerConfig spannerConfig) {
   private static SpannerAccessor createAndConnect(SpannerConfig spannerConfig) {
     SpannerOptions.Builder builder = SpannerOptions.newBuilder();
 
-    ValueProvider<Duration> commitDeadline = spannerConfig.getCommitDeadline();
-    if (commitDeadline != null && commitDeadline.get().getMillis() > 0) {
+    // Set retryable codes
+    if (spannerConfig.getRetryableCodes() != null) {

Review comment:
       I think that it might be unintuitive that the retryable codes set by the user apply only to the commit settings and the executeStreamingSqlSettings.
   
   Is there any way we could explore settings for specific methods in the `SpannerConfig`? Maybe a mapping of `Method` to retryable codes / retry settings?
   




-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] thiagotnunes commented on pull request #16846: test: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
thiagotnunes commented on pull request #16846:
URL: https://github.com/apache/beam/pull/16846#issuecomment-1039887585


   You should change the title to match the JIRA ticket `[BEAM-12164]: <title>`


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] zoercai commented on pull request #16846: [BEAM-12164]: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
zoercai commented on pull request #16846:
URL: https://github.com/apache/beam/pull/16846#issuecomment-1042523934


   Hey @pabloem could you please review this PR?


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] codecov[bot] edited a comment on pull request #16846: [BEAM-12164]: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16846:
URL: https://github.com/apache/beam/pull/16846#issuecomment-1039895718


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#16846](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (16ce8d8) into [master](https://codecov.io/gh/apache/beam/commit/8bf68ad5023c30cd56ec63d2084017b4a547fe48?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8bf68ad) will **increase** coverage by `8.95%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16846/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #16846      +/-   ##
   ==========================================
   + Coverage   74.63%   83.59%   +8.95%     
   ==========================================
     Files         655      452     -203     
     Lines       82237    62291   -19946     
   ==========================================
   - Hits        61380    52073    -9307     
   + Misses      19861    10218    -9643     
   + Partials      996        0     -996     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/utils/interactive\_utils.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvaW50ZXJhY3RpdmVfdXRpbHMucHk=) | `87.80% <0.00%> (-7.32%)` | :arrow_down: |
   | [sdks/python/apache\_beam/io/jdbc.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vamRiYy5weQ==) | `78.26% <0.00%> (-6.74%)` | :arrow_down: |
   | [...s/python/apache\_beam/runners/portability/stager.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9zdGFnZXIucHk=) | `85.54% <0.00%> (-2.82%)` | :arrow_down: |
   | [.../python/apache\_beam/transforms/periodicsequence.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9wZXJpb2RpY3NlcXVlbmNlLnB5) | `96.72% <0.00%> (-1.64%)` | :arrow_down: |
   | [...pache\_beam/runners/interactive/interactive\_beam.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9iZWFtLnB5) | `76.41% <0.00%> (-0.95%)` | :arrow_down: |
   | [...runners/interactive/display/pcoll\_visualization.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9kaXNwbGF5L3Bjb2xsX3Zpc3VhbGl6YXRpb24ucHk=) | `85.85% <0.00%> (-0.51%)` | :arrow_down: |
   | [...eam/runners/interactive/interactive\_environment.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9lbnZpcm9ubWVudC5weQ==) | `90.09% <0.00%> (-0.31%)` | :arrow_down: |
   | [setup.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2V0dXAucHk=) | `0.00% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/transforms/core.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9jb3JlLnB5) | `92.64% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/portability/common\_urns.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvY29tbW9uX3VybnMucHk=) | `100.00% <0.00%> (ø)` | |
   | ... and [217 more](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [8bf68ad...16ce8d8](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] codecov[bot] commented on pull request #16846: [BEAM-12164]: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #16846:
URL: https://github.com/apache/beam/pull/16846#issuecomment-1039895718


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#16846](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (16ce8d8) into [master](https://codecov.io/gh/apache/beam/commit/8bf68ad5023c30cd56ec63d2084017b4a547fe48?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8bf68ad) will **increase** coverage by `8.95%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16846/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #16846      +/-   ##
   ==========================================
   + Coverage   74.63%   83.59%   +8.95%     
   ==========================================
     Files         655      452     -203     
     Lines       82237    62291   -19946     
   ==========================================
   - Hits        61380    52073    -9307     
   + Misses      19861    10218    -9643     
   + Partials      996        0     -996     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/utils/interactive\_utils.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvaW50ZXJhY3RpdmVfdXRpbHMucHk=) | `87.80% <0.00%> (-7.32%)` | :arrow_down: |
   | [sdks/python/apache\_beam/io/jdbc.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vamRiYy5weQ==) | `78.26% <0.00%> (-6.74%)` | :arrow_down: |
   | [...s/python/apache\_beam/runners/portability/stager.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9zdGFnZXIucHk=) | `85.54% <0.00%> (-2.82%)` | :arrow_down: |
   | [.../python/apache\_beam/transforms/periodicsequence.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9wZXJpb2RpY3NlcXVlbmNlLnB5) | `96.72% <0.00%> (-1.64%)` | :arrow_down: |
   | [...pache\_beam/runners/interactive/interactive\_beam.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9iZWFtLnB5) | `76.41% <0.00%> (-0.95%)` | :arrow_down: |
   | [...runners/interactive/display/pcoll\_visualization.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9kaXNwbGF5L3Bjb2xsX3Zpc3VhbGl6YXRpb24ucHk=) | `85.85% <0.00%> (-0.51%)` | :arrow_down: |
   | [...eam/runners/interactive/interactive\_environment.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9lbnZpcm9ubWVudC5weQ==) | `90.09% <0.00%> (-0.31%)` | :arrow_down: |
   | [setup.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2V0dXAucHk=) | `0.00% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/transforms/core.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9jb3JlLnB5) | `92.64% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/portability/common\_urns.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvY29tbW9uX3VybnMucHk=) | `100.00% <0.00%> (ø)` | |
   | ... and [217 more](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [8bf68ad...16ce8d8](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] pabloem merged pull request #16846: [BEAM-12164]: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
pabloem merged pull request #16846:
URL: https://github.com/apache/beam/pull/16846


   


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] codecov[bot] edited a comment on pull request #16846: [BEAM-12164]: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16846:
URL: https://github.com/apache/beam/pull/16846#issuecomment-1039895718


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#16846](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (361e6da) into [master](https://codecov.io/gh/apache/beam/commit/8bf68ad5023c30cd56ec63d2084017b4a547fe48?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8bf68ad) will **increase** coverage by `8.97%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16846/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #16846      +/-   ##
   ==========================================
   + Coverage   74.63%   83.61%   +8.97%     
   ==========================================
     Files         655      452     -203     
     Lines       82237    62292   -19945     
   ==========================================
   - Hits        61380    52085    -9295     
   + Misses      19861    10207    -9654     
   + Partials      996        0     -996     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/io/jdbc.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vamRiYy5weQ==) | `78.26% <0.00%> (-6.74%)` | :arrow_down: |
   | [...s/python/apache\_beam/runners/portability/stager.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9zdGFnZXIucHk=) | `85.54% <0.00%> (-2.82%)` | :arrow_down: |
   | [setup.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2V0dXAucHk=) | `0.00% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/transforms/core.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9jb3JlLnB5) | `92.64% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/portability/common\_urns.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvY29tbW9uX3VybnMucHk=) | `100.00% <0.00%> (ø)` | |
   | [...python/apache\_beam/runners/direct/direct\_runner.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kaXJlY3QvZGlyZWN0X3J1bm5lci5weQ==) | `93.92% <0.00%> (ø)` | |
   | [...on/apache\_beam/examples/dataframe/flight\_delays.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZXhhbXBsZXMvZGF0YWZyYW1lL2ZsaWdodF9kZWxheXMucHk=) | `27.90% <0.00%> (ø)` | |
   | [...apache\_beam/portability/api/beam\_runner\_api\_pb2.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvYXBpL2JlYW1fcnVubmVyX2FwaV9wYjIucHk=) | `100.00% <0.00%> (ø)` | |
   | [...apache\_beam/typehints/native\_type\_compatibility.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHlwZWhpbnRzL25hdGl2ZV90eXBlX2NvbXBhdGliaWxpdHkucHk=) | `86.57% <0.00%> (ø)` | |
   | [...e\_beam/portability/api/beam\_runner\_api\_pb2\_urns.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvYXBpL2JlYW1fcnVubmVyX2FwaV9wYjJfdXJucy5weQ==) | `100.00% <0.00%> (ø)` | |
   | ... and [215 more](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [8bf68ad...361e6da](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] zoercai commented on a change in pull request #16846: test: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
zoercai commented on a change in pull request #16846:
URL: https://github.com/apache/beam/pull/16846#discussion_r806452368



##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java
##########
@@ -107,34 +108,56 @@ public static SpannerAccessor getOrCreate(SpannerConfig spannerConfig) {
   private static SpannerAccessor createAndConnect(SpannerConfig spannerConfig) {
     SpannerOptions.Builder builder = SpannerOptions.newBuilder();
 
-    ValueProvider<Duration> commitDeadline = spannerConfig.getCommitDeadline();
-    if (commitDeadline != null && commitDeadline.get().getMillis() > 0) {
+    // Set retryable codes
+    if (spannerConfig.getRetryableCodes() != null) {

Review comment:
       I actually started this work from trying to put in `SpannerStubSettings` rather than the separate `retryableCodes` and `retrySettings` currently here, but `SpannerStubSettings` is not serialisable and I couldn't get it to work after trying for hours.
   
   I have changed this part so that the retryable codes get applied to all API methods rather than just the commit and executeStreamingSql though. Let me know if that doesn't work :) 




-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] zoercai commented on a change in pull request #16846: test: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
zoercai commented on a change in pull request #16846:
URL: https://github.com/apache/beam/pull/16846#discussion_r806446372



##########
File path: sdks/java/io/google-cloud-platform/build.gradle
##########
@@ -146,6 +147,7 @@ dependencies {
   testImplementation library.java.powermock
   testImplementation library.java.powermock_mockito
   testImplementation library.java.joda_time
+  testImplementation "com.google.cloud:google-cloud-spanner:6.17.4:tests"

Review comment:
       It doesn't work if I remove the version, but I've moved this to `BeamModulePlugin.groovy` with the versions defined by variables




-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] codecov[bot] edited a comment on pull request #16846: [BEAM-12164]: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16846:
URL: https://github.com/apache/beam/pull/16846#issuecomment-1039895718


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#16846](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (16ce8d8) into [master](https://codecov.io/gh/apache/beam/commit/8bf68ad5023c30cd56ec63d2084017b4a547fe48?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8bf68ad) will **increase** coverage by `8.95%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16846/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #16846      +/-   ##
   ==========================================
   + Coverage   74.63%   83.59%   +8.95%     
   ==========================================
     Files         655      452     -203     
     Lines       82237    62291   -19946     
   ==========================================
   - Hits        61380    52073    -9307     
   + Misses      19861    10218    -9643     
   + Partials      996        0     -996     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/utils/interactive\_utils.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvaW50ZXJhY3RpdmVfdXRpbHMucHk=) | `87.80% <0.00%> (-7.32%)` | :arrow_down: |
   | [sdks/python/apache\_beam/io/jdbc.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vamRiYy5weQ==) | `78.26% <0.00%> (-6.74%)` | :arrow_down: |
   | [...s/python/apache\_beam/runners/portability/stager.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9zdGFnZXIucHk=) | `85.54% <0.00%> (-2.82%)` | :arrow_down: |
   | [.../python/apache\_beam/transforms/periodicsequence.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9wZXJpb2RpY3NlcXVlbmNlLnB5) | `96.72% <0.00%> (-1.64%)` | :arrow_down: |
   | [...pache\_beam/runners/interactive/interactive\_beam.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9iZWFtLnB5) | `76.41% <0.00%> (-0.95%)` | :arrow_down: |
   | [...runners/interactive/display/pcoll\_visualization.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9kaXNwbGF5L3Bjb2xsX3Zpc3VhbGl6YXRpb24ucHk=) | `85.85% <0.00%> (-0.51%)` | :arrow_down: |
   | [...eam/runners/interactive/interactive\_environment.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9lbnZpcm9ubWVudC5weQ==) | `90.09% <0.00%> (-0.31%)` | :arrow_down: |
   | [setup.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2V0dXAucHk=) | `0.00% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/transforms/core.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9jb3JlLnB5) | `92.64% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/portability/common\_urns.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvY29tbW9uX3VybnMucHk=) | `100.00% <0.00%> (ø)` | |
   | ... and [217 more](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [8bf68ad...16ce8d8](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] codecov[bot] edited a comment on pull request #16846: [BEAM-12164]: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16846:
URL: https://github.com/apache/beam/pull/16846#issuecomment-1039895718


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#16846](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0e12e60) into [master](https://codecov.io/gh/apache/beam/commit/8bf68ad5023c30cd56ec63d2084017b4a547fe48?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8bf68ad) will **increase** coverage by `8.99%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16846/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #16846      +/-   ##
   ==========================================
   + Coverage   74.63%   83.63%   +8.99%     
   ==========================================
     Files         655      453     -202     
     Lines       82237    62471   -19766     
   ==========================================
   - Hits        61380    52245    -9135     
   + Misses      19861    10226    -9635     
   + Partials      996        0     -996     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/io/jdbc.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vamRiYy5weQ==) | `78.26% <0.00%> (-6.74%)` | :arrow_down: |
   | [...s/python/apache\_beam/runners/portability/stager.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9zdGFnZXIucHk=) | `85.54% <0.00%> (-2.82%)` | :arrow_down: |
   | [...apache\_beam/runners/portability/portable\_runner.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9wb3J0YWJsZV9ydW5uZXIucHk=) | `75.87% <0.00%> (-0.27%)` | :arrow_down: |
   | [setup.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2V0dXAucHk=) | `0.00% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/transforms/core.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9jb3JlLnB5) | `92.64% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/portability/common\_urns.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvY29tbW9uX3VybnMucHk=) | `100.00% <0.00%> (ø)` | |
   | [...python/apache\_beam/runners/direct/direct\_runner.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kaXJlY3QvZGlyZWN0X3J1bm5lci5weQ==) | `93.92% <0.00%> (ø)` | |
   | [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.39% <0.00%> (ø)` | |
   | [...on/apache\_beam/examples/dataframe/flight\_delays.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZXhhbXBsZXMvZGF0YWZyYW1lL2ZsaWdodF9kZWxheXMucHk=) | `27.90% <0.00%> (ø)` | |
   | [...apache\_beam/portability/api/beam\_runner\_api\_pb2.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvYXBpL2JlYW1fcnVubmVyX2FwaV9wYjIucHk=) | `100.00% <0.00%> (ø)` | |
   | ... and [220 more](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [8bf68ad...0e12e60](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] zoercai commented on a change in pull request #16846: test: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
zoercai commented on a change in pull request #16846:
URL: https://github.com/apache/beam/pull/16846#discussion_r806446372



##########
File path: sdks/java/io/google-cloud-platform/build.gradle
##########
@@ -146,6 +147,7 @@ dependencies {
   testImplementation library.java.powermock
   testImplementation library.java.powermock_mockito
   testImplementation library.java.joda_time
+  testImplementation "com.google.cloud:google-cloud-spanner:6.17.4:tests"

Review comment:
       It doesn't work if I remove the version, but I've moved this to `BeamModulePlugin.groovy` with the versions defined by variables

##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java
##########
@@ -107,34 +108,56 @@ public static SpannerAccessor getOrCreate(SpannerConfig spannerConfig) {
   private static SpannerAccessor createAndConnect(SpannerConfig spannerConfig) {
     SpannerOptions.Builder builder = SpannerOptions.newBuilder();
 
-    ValueProvider<Duration> commitDeadline = spannerConfig.getCommitDeadline();
-    if (commitDeadline != null && commitDeadline.get().getMillis() > 0) {
+    // Set retryable codes
+    if (spannerConfig.getRetryableCodes() != null) {

Review comment:
       I actually started this work from trying to put in `SpannerStubSettings` rather than the separate `retryableCodes` and `retrySettings` currently here, but `SpannerStubSettings` is not serialisable and I couldn't get it to work after trying for hours.
   
   I have changed this part so that the retryable codes get applied to all API methods rather than just the commit and executeStreamingSql though. Let me know if that doesn't work :) 

##########
File path: sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/SpannerChangeStreamErrorTest.java
##########
@@ -0,0 +1,494 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.spanner.changestreams;
+
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_CREATED_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_END_TIMESTAMP;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_FINISHED_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_HEARTBEAT_MILLIS;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_PARENT_TOKENS;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_PARTITION_TOKEN;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_RUNNING_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_SCHEDULED_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_START_TIMESTAMP;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_STATE;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_WATERMARK;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.google.api.gax.grpc.testing.MockServiceHelper;
+import com.google.api.gax.retrying.RetrySettings;
+import com.google.cloud.Timestamp;
+import com.google.cloud.spanner.ErrorCode;
+import com.google.cloud.spanner.MockSpannerServiceImpl;
+import com.google.cloud.spanner.MockSpannerServiceImpl.SimulatedExecutionTime;
+import com.google.cloud.spanner.MockSpannerServiceImpl.StatementResult;
+import com.google.cloud.spanner.Statement;
+import com.google.protobuf.ListValue;
+import com.google.protobuf.NullValue;
+import com.google.protobuf.Value;
+import com.google.spanner.v1.ExecuteSqlRequest;
+import com.google.spanner.v1.ResultSet;
+import com.google.spanner.v1.ResultSetMetadata;
+import com.google.spanner.v1.StructType;
+import com.google.spanner.v1.StructType.Field;
+import com.google.spanner.v1.Type;
+import com.google.spanner.v1.TypeCode;
+import io.grpc.Status;
+import java.io.Serializable;
+import java.util.Collections;
+import org.apache.beam.sdk.Pipeline.PipelineExecutionException;
+import org.apache.beam.sdk.io.gcp.spanner.SpannerConfig;
+import org.apache.beam.sdk.io.gcp.spanner.SpannerIO;
+import org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.DaoFactory;
+import org.apache.beam.sdk.io.gcp.spanner.changestreams.model.PartitionMetadata.State;
+import org.apache.beam.sdk.options.ValueProvider.StaticValueProvider;
+import org.apache.beam.sdk.testing.TestPipeline;
+import org.apache.beam.vendor.guava.v26_0_jre.com.google.common.collect.ImmutableSet;
+import org.hamcrest.Matchers;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public class SpannerChangeStreamErrorTest implements Serializable {
+
+  public static final String SPANNER_HOST = "my-host";
+  private static final String TEST_PROJECT = "my-project";
+  private static final String TEST_INSTANCE = "my-instance";
+  private static final String TEST_DATABASE = "my-database";
+  private static final String TEST_TABLE = "my-metadata-table";
+  private static final String TEST_CHANGE_STREAM = "my-change-stream";
+
+  @Rule
+  public final transient TestPipeline pipeline =
+      TestPipeline.create().enableAbandonedNodeEnforcement(false);
+
+  @Rule public final transient ExpectedException thrown = ExpectedException.none();
+
+  private MockSpannerServiceImpl mockSpannerService;
+  private MockServiceHelper serviceHelper;
+
+  @Before
+  public void setUp() throws Exception {
+    mockSpannerService = new MockSpannerServiceImpl();
+    serviceHelper =
+        new MockServiceHelper(SPANNER_HOST, Collections.singletonList(mockSpannerService));
+    serviceHelper.start();
+    serviceHelper.reset();
+  }
+
+  @After
+  public void tearDown() throws NoSuchFieldException, IllegalAccessException {
+    serviceHelper.reset();
+    serviceHelper.stop();
+    mockSpannerService.reset();
+    resetDaoFactoryFields();
+  }
+
+  @Test
+  public void testResourceExhaustedDoesNotRetry() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofStickyException(Status.RESOURCE_EXHAUSTED.asRuntimeException()));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.RESOURCE_EXHAUSTED.name());
+    }
+  }
+
+  @Test
+  public void testUnavailableExceptionRetries() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofExceptions(
+            ImmutableSet.of(
+                Status.UNAVAILABLE.asRuntimeException(),
+                Status.RESOURCE_EXHAUSTED.asRuntimeException())));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      assertThat(
+          mockSpannerService.countRequestsOfType(ExecuteSqlRequest.class), Matchers.greaterThan(1));
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.RESOURCE_EXHAUSTED.name());
+    }
+  }
+
+  @Test
+  public void testAbortedExceptionRetries() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofStickyException(Status.ABORTED.asRuntimeException()));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      assertThat(
+          mockSpannerService.countRequestsOfType(ExecuteSqlRequest.class), Matchers.greaterThan(1));
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.ABORTED.name());
+    }
+  }
+
+  @Test
+  public void testUnknownExceptionDoesNotRetry() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofStickyException(Status.UNKNOWN.asRuntimeException()));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      assertThat(
+          mockSpannerService.countRequestsOfType(ExecuteSqlRequest.class), Matchers.equalTo(1));
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.UNKNOWN.name());
+    }
+  }
+
+  @Test
+  public void testInvalidRecordReceived() {
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+
+    mockTableExists();
+    ResultSet getPartitionResultSet = mockGetParentPartition(now, after3Seconds);
+    mockGetWatermark(now);
+    mockGetPartitionsAfter(
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds(), now.getNanos() + 1000),
+        getPartitionResultSet);
+    mockGetPartitionsAfter(
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds(), now.getNanos() - 1000),
+        getPartitionResultSet);
+
+    Statement changeStreamQueryStatement =
+        Statement.newBuilder(
+                "SELECT * FROM READ_my-change-stream(   start_timestamp => @startTimestamp,   end_timestamp => @endTimestamp,   partition_token => @partitionToken,   read_options => null,   heartbeat_milliseconds => @heartbeatMillis)")

Review comment:
       This is actually how it looks in the actual request statement, I believe the test will fail if I take out the spaces — https://github.com/googleapis/java-spanner/blob/main/google-cloud-spanner/src/test/java/com/google/cloud/spanner/MockSpannerServiceImpl.java#L113




-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] codecov[bot] edited a comment on pull request #16846: [BEAM-12164]: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16846:
URL: https://github.com/apache/beam/pull/16846#issuecomment-1039895718


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#16846](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (361e6da) into [master](https://codecov.io/gh/apache/beam/commit/8bf68ad5023c30cd56ec63d2084017b4a547fe48?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8bf68ad) will **increase** coverage by `8.97%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16846/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #16846      +/-   ##
   ==========================================
   + Coverage   74.63%   83.61%   +8.97%     
   ==========================================
     Files         655      452     -203     
     Lines       82237    62292   -19945     
   ==========================================
   - Hits        61380    52085    -9295     
   + Misses      19861    10207    -9654     
   + Partials      996        0     -996     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/io/jdbc.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vamRiYy5weQ==) | `78.26% <0.00%> (-6.74%)` | :arrow_down: |
   | [...s/python/apache\_beam/runners/portability/stager.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9zdGFnZXIucHk=) | `85.54% <0.00%> (-2.82%)` | :arrow_down: |
   | [setup.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2V0dXAucHk=) | `0.00% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/transforms/core.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9jb3JlLnB5) | `92.64% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/portability/common\_urns.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvY29tbW9uX3VybnMucHk=) | `100.00% <0.00%> (ø)` | |
   | [...python/apache\_beam/runners/direct/direct\_runner.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kaXJlY3QvZGlyZWN0X3J1bm5lci5weQ==) | `93.92% <0.00%> (ø)` | |
   | [...on/apache\_beam/examples/dataframe/flight\_delays.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZXhhbXBsZXMvZGF0YWZyYW1lL2ZsaWdodF9kZWxheXMucHk=) | `27.90% <0.00%> (ø)` | |
   | [...apache\_beam/portability/api/beam\_runner\_api\_pb2.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvYXBpL2JlYW1fcnVubmVyX2FwaV9wYjIucHk=) | `100.00% <0.00%> (ø)` | |
   | [...apache\_beam/typehints/native\_type\_compatibility.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHlwZWhpbnRzL25hdGl2ZV90eXBlX2NvbXBhdGliaWxpdHkucHk=) | `86.57% <0.00%> (ø)` | |
   | [...e\_beam/portability/api/beam\_runner\_api\_pb2\_urns.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvYXBpL2JlYW1fcnVubmVyX2FwaV9wYjJfdXJucy5weQ==) | `100.00% <0.00%> (ø)` | |
   | ... and [215 more](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [8bf68ad...361e6da](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] codecov[bot] edited a comment on pull request #16846: [BEAM-12164]: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16846:
URL: https://github.com/apache/beam/pull/16846#issuecomment-1039895718


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#16846](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (361e6da) into [master](https://codecov.io/gh/apache/beam/commit/8bf68ad5023c30cd56ec63d2084017b4a547fe48?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8bf68ad) will **increase** coverage by `8.97%`.
   > The diff coverage is `n/a`.
   
   > :exclamation: Current head 361e6da differs from pull request most recent head 0e12e60. Consider uploading reports for the commit 0e12e60 to get more accurate results
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16846/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #16846      +/-   ##
   ==========================================
   + Coverage   74.63%   83.61%   +8.97%     
   ==========================================
     Files         655      452     -203     
     Lines       82237    62292   -19945     
   ==========================================
   - Hits        61380    52085    -9295     
   + Misses      19861    10207    -9654     
   + Partials      996        0     -996     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/io/jdbc.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vamRiYy5weQ==) | `78.26% <0.00%> (-6.74%)` | :arrow_down: |
   | [...s/python/apache\_beam/runners/portability/stager.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9zdGFnZXIucHk=) | `85.54% <0.00%> (-2.82%)` | :arrow_down: |
   | [setup.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2V0dXAucHk=) | `0.00% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/transforms/core.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9jb3JlLnB5) | `92.64% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/portability/common\_urns.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvY29tbW9uX3VybnMucHk=) | `100.00% <0.00%> (ø)` | |
   | [...python/apache\_beam/runners/direct/direct\_runner.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kaXJlY3QvZGlyZWN0X3J1bm5lci5weQ==) | `93.92% <0.00%> (ø)` | |
   | [...on/apache\_beam/examples/dataframe/flight\_delays.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZXhhbXBsZXMvZGF0YWZyYW1lL2ZsaWdodF9kZWxheXMucHk=) | `27.90% <0.00%> (ø)` | |
   | [...apache\_beam/portability/api/beam\_runner\_api\_pb2.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvYXBpL2JlYW1fcnVubmVyX2FwaV9wYjIucHk=) | `100.00% <0.00%> (ø)` | |
   | [...apache\_beam/typehints/native\_type\_compatibility.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHlwZWhpbnRzL25hdGl2ZV90eXBlX2NvbXBhdGliaWxpdHkucHk=) | `86.57% <0.00%> (ø)` | |
   | [...e\_beam/portability/api/beam\_runner\_api\_pb2\_urns.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvYXBpL2JlYW1fcnVubmVyX2FwaV9wYjJfdXJucy5weQ==) | `100.00% <0.00%> (ø)` | |
   | ... and [215 more](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [8bf68ad...0e12e60](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] codecov[bot] edited a comment on pull request #16846: [BEAM-12164]: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16846:
URL: https://github.com/apache/beam/pull/16846#issuecomment-1039895718


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#16846](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0e12e60) into [master](https://codecov.io/gh/apache/beam/commit/8bf68ad5023c30cd56ec63d2084017b4a547fe48?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8bf68ad) will **increase** coverage by `8.99%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16846/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #16846      +/-   ##
   ==========================================
   + Coverage   74.63%   83.63%   +8.99%     
   ==========================================
     Files         655      453     -202     
     Lines       82237    62471   -19766     
   ==========================================
   - Hits        61380    52245    -9135     
   + Misses      19861    10226    -9635     
   + Partials      996        0     -996     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/io/jdbc.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vamRiYy5weQ==) | `78.26% <0.00%> (-6.74%)` | :arrow_down: |
   | [...s/python/apache\_beam/runners/portability/stager.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9zdGFnZXIucHk=) | `85.54% <0.00%> (-2.82%)` | :arrow_down: |
   | [...apache\_beam/runners/portability/portable\_runner.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9wb3J0YWJsZV9ydW5uZXIucHk=) | `75.87% <0.00%> (-0.27%)` | :arrow_down: |
   | [setup.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2V0dXAucHk=) | `0.00% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/transforms/core.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9jb3JlLnB5) | `92.64% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/portability/common\_urns.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvY29tbW9uX3VybnMucHk=) | `100.00% <0.00%> (ø)` | |
   | [...python/apache\_beam/runners/direct/direct\_runner.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kaXJlY3QvZGlyZWN0X3J1bm5lci5weQ==) | `93.92% <0.00%> (ø)` | |
   | [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.39% <0.00%> (ø)` | |
   | [...on/apache\_beam/examples/dataframe/flight\_delays.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZXhhbXBsZXMvZGF0YWZyYW1lL2ZsaWdodF9kZWxheXMucHk=) | `27.90% <0.00%> (ø)` | |
   | [...apache\_beam/portability/api/beam\_runner\_api\_pb2.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvYXBpL2JlYW1fcnVubmVyX2FwaV9wYjIucHk=) | `100.00% <0.00%> (ø)` | |
   | ... and [220 more](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [8bf68ad...0e12e60](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] pabloem commented on a change in pull request #16846: [BEAM-12164]: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
pabloem commented on a change in pull request #16846:
URL: https://github.com/apache/beam/pull/16846#discussion_r814525377



##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java
##########
@@ -151,6 +177,10 @@ private static SpannerAccessor createAndConnect(SpannerConfig spannerConfig) {
     ValueProvider<String> emulatorHost = spannerConfig.getEmulatorHost();
     if (emulatorHost != null) {
       builder.setEmulatorHost(emulatorHost.get());
+      if (spannerConfig.getIsLocalChannelProvider() != null
+          && spannerConfig.getIsLocalChannelProvider().get()) {
+        builder.setChannelProvider(LocalChannelProvider.create(emulatorHost.get()));

Review comment:
       is there any chance of passing the LocalChannelProvider from the test to avoid depending on gax_grpc_test in the main package? It's not mandatory, but it would make life easier for like 1% of users, who, may have version clashes with that at some point : )




-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] codecov[bot] edited a comment on pull request #16846: [BEAM-12164]: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16846:
URL: https://github.com/apache/beam/pull/16846#issuecomment-1039895718


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#16846](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (361e6da) into [master](https://codecov.io/gh/apache/beam/commit/8bf68ad5023c30cd56ec63d2084017b4a547fe48?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8bf68ad) will **increase** coverage by `8.97%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16846/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #16846      +/-   ##
   ==========================================
   + Coverage   74.63%   83.61%   +8.97%     
   ==========================================
     Files         655      452     -203     
     Lines       82237    62292   -19945     
   ==========================================
   - Hits        61380    52085    -9295     
   + Misses      19861    10207    -9654     
   + Partials      996        0     -996     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/io/jdbc.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vamRiYy5weQ==) | `78.26% <0.00%> (-6.74%)` | :arrow_down: |
   | [...s/python/apache\_beam/runners/portability/stager.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9zdGFnZXIucHk=) | `85.54% <0.00%> (-2.82%)` | :arrow_down: |
   | [setup.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2V0dXAucHk=) | `0.00% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/transforms/core.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9jb3JlLnB5) | `92.64% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/portability/common\_urns.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvY29tbW9uX3VybnMucHk=) | `100.00% <0.00%> (ø)` | |
   | [...python/apache\_beam/runners/direct/direct\_runner.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kaXJlY3QvZGlyZWN0X3J1bm5lci5weQ==) | `93.92% <0.00%> (ø)` | |
   | [...on/apache\_beam/examples/dataframe/flight\_delays.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZXhhbXBsZXMvZGF0YWZyYW1lL2ZsaWdodF9kZWxheXMucHk=) | `27.90% <0.00%> (ø)` | |
   | [...apache\_beam/portability/api/beam\_runner\_api\_pb2.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvYXBpL2JlYW1fcnVubmVyX2FwaV9wYjIucHk=) | `100.00% <0.00%> (ø)` | |
   | [...apache\_beam/typehints/native\_type\_compatibility.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHlwZWhpbnRzL25hdGl2ZV90eXBlX2NvbXBhdGliaWxpdHkucHk=) | `86.57% <0.00%> (ø)` | |
   | [...e\_beam/portability/api/beam\_runner\_api\_pb2\_urns.py](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvYXBpL2JlYW1fcnVubmVyX2FwaV9wYjJfdXJucy5weQ==) | `100.00% <0.00%> (ø)` | |
   | ... and [215 more](https://codecov.io/gh/apache/beam/pull/16846/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [8bf68ad...361e6da](https://codecov.io/gh/apache/beam/pull/16846?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] thiagotnunes commented on a change in pull request #16846: [BEAM-12164]: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
thiagotnunes commented on a change in pull request #16846:
URL: https://github.com/apache/beam/pull/16846#discussion_r808600643



##########
File path: buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy
##########
@@ -454,8 +454,12 @@ class BeamModulePlugin implements Plugin<Project> {
     def checkerframework_version = "3.10.0"
     def classgraph_version = "4.8.104"
     def errorprone_version = "2.10.0"
+    // Try to keep gax_version consistent with gax-grpc version in google_cloud_platform_libraries_bom
+    def gax_version = "2.8.1"

Review comment:
       That is ok, thanks for trying




-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] pabloem commented on pull request #16846: [BEAM-12164]: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #16846:
URL: https://github.com/apache/beam/pull/16846#issuecomment-1050581027


   once again I'm sorry about the delay. Taking a look now!


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] thiagotnunes commented on a change in pull request #16846: test: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
thiagotnunes commented on a change in pull request #16846:
URL: https://github.com/apache/beam/pull/16846#discussion_r806467293



##########
File path: sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/SpannerChangeStreamErrorTest.java
##########
@@ -0,0 +1,494 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.spanner.changestreams;
+
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_CREATED_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_END_TIMESTAMP;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_FINISHED_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_HEARTBEAT_MILLIS;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_PARENT_TOKENS;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_PARTITION_TOKEN;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_RUNNING_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_SCHEDULED_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_START_TIMESTAMP;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_STATE;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_WATERMARK;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.google.api.gax.grpc.testing.MockServiceHelper;
+import com.google.api.gax.retrying.RetrySettings;
+import com.google.cloud.Timestamp;
+import com.google.cloud.spanner.ErrorCode;
+import com.google.cloud.spanner.MockSpannerServiceImpl;
+import com.google.cloud.spanner.MockSpannerServiceImpl.SimulatedExecutionTime;
+import com.google.cloud.spanner.MockSpannerServiceImpl.StatementResult;
+import com.google.cloud.spanner.Statement;
+import com.google.protobuf.ListValue;
+import com.google.protobuf.NullValue;
+import com.google.protobuf.Value;
+import com.google.spanner.v1.ExecuteSqlRequest;
+import com.google.spanner.v1.ResultSet;
+import com.google.spanner.v1.ResultSetMetadata;
+import com.google.spanner.v1.StructType;
+import com.google.spanner.v1.StructType.Field;
+import com.google.spanner.v1.Type;
+import com.google.spanner.v1.TypeCode;
+import io.grpc.Status;
+import java.io.Serializable;
+import java.util.Collections;
+import org.apache.beam.sdk.Pipeline.PipelineExecutionException;
+import org.apache.beam.sdk.io.gcp.spanner.SpannerConfig;
+import org.apache.beam.sdk.io.gcp.spanner.SpannerIO;
+import org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.DaoFactory;
+import org.apache.beam.sdk.io.gcp.spanner.changestreams.model.PartitionMetadata.State;
+import org.apache.beam.sdk.options.ValueProvider.StaticValueProvider;
+import org.apache.beam.sdk.testing.TestPipeline;
+import org.apache.beam.vendor.guava.v26_0_jre.com.google.common.collect.ImmutableSet;
+import org.hamcrest.Matchers;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public class SpannerChangeStreamErrorTest implements Serializable {
+
+  public static final String SPANNER_HOST = "my-host";
+  private static final String TEST_PROJECT = "my-project";
+  private static final String TEST_INSTANCE = "my-instance";
+  private static final String TEST_DATABASE = "my-database";
+  private static final String TEST_TABLE = "my-metadata-table";
+  private static final String TEST_CHANGE_STREAM = "my-change-stream";
+
+  @Rule
+  public final transient TestPipeline pipeline =
+      TestPipeline.create().enableAbandonedNodeEnforcement(false);
+
+  @Rule public final transient ExpectedException thrown = ExpectedException.none();
+
+  private MockSpannerServiceImpl mockSpannerService;
+  private MockServiceHelper serviceHelper;
+
+  @Before
+  public void setUp() throws Exception {
+    mockSpannerService = new MockSpannerServiceImpl();
+    serviceHelper =
+        new MockServiceHelper(SPANNER_HOST, Collections.singletonList(mockSpannerService));
+    serviceHelper.start();
+    serviceHelper.reset();
+  }
+
+  @After
+  public void tearDown() throws NoSuchFieldException, IllegalAccessException {
+    serviceHelper.reset();
+    serviceHelper.stop();
+    mockSpannerService.reset();
+    resetDaoFactoryFields();
+  }
+
+  @Test
+  public void testResourceExhaustedDoesNotRetry() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofStickyException(Status.RESOURCE_EXHAUSTED.asRuntimeException()));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.RESOURCE_EXHAUSTED.name());
+    }
+  }
+
+  @Test
+  public void testUnavailableExceptionRetries() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofExceptions(
+            ImmutableSet.of(
+                Status.UNAVAILABLE.asRuntimeException(),
+                Status.RESOURCE_EXHAUSTED.asRuntimeException())));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      assertThat(
+          mockSpannerService.countRequestsOfType(ExecuteSqlRequest.class), Matchers.greaterThan(1));
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.RESOURCE_EXHAUSTED.name());
+    }
+  }
+
+  @Test
+  public void testAbortedExceptionRetries() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofStickyException(Status.ABORTED.asRuntimeException()));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      assertThat(
+          mockSpannerService.countRequestsOfType(ExecuteSqlRequest.class), Matchers.greaterThan(1));
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.ABORTED.name());
+    }
+  }
+
+  @Test
+  public void testUnknownExceptionDoesNotRetry() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofStickyException(Status.UNKNOWN.asRuntimeException()));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      assertThat(
+          mockSpannerService.countRequestsOfType(ExecuteSqlRequest.class), Matchers.equalTo(1));
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.UNKNOWN.name());
+    }
+  }
+
+  @Test
+  public void testInvalidRecordReceived() {
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+
+    mockTableExists();
+    ResultSet getPartitionResultSet = mockGetParentPartition(now, after3Seconds);
+    mockGetWatermark(now);
+    mockGetPartitionsAfter(
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds(), now.getNanos() + 1000),
+        getPartitionResultSet);
+    mockGetPartitionsAfter(
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds(), now.getNanos() - 1000),
+        getPartitionResultSet);
+
+    Statement changeStreamQueryStatement =
+        Statement.newBuilder(
+                "SELECT * FROM READ_my-change-stream(   start_timestamp => @startTimestamp,   end_timestamp => @endTimestamp,   partition_token => @partitionToken,   read_options => null,   heartbeat_milliseconds => @heartbeatMillis)")

Review comment:
       nit: we could remove the extra spacing here (probably spotless added it when merging lines into one)

##########
File path: buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy
##########
@@ -454,8 +454,12 @@ class BeamModulePlugin implements Plugin<Project> {
     def checkerframework_version = "3.10.0"
     def classgraph_version = "4.8.104"
     def errorprone_version = "2.10.0"
+    // Try to keep gax_version consistent with gax-grpc version in google_cloud_platform_libraries_bom
+    def gax_version = "2.8.1"

Review comment:
       I think that this one will work without the version as the BOM defines a version for it (https://mvnrepository.com/artifact/com.google.cloud/libraries-bom/24.2.0)

##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java
##########
@@ -107,34 +108,56 @@ public static SpannerAccessor getOrCreate(SpannerConfig spannerConfig) {
   private static SpannerAccessor createAndConnect(SpannerConfig spannerConfig) {
     SpannerOptions.Builder builder = SpannerOptions.newBuilder();
 
-    ValueProvider<Duration> commitDeadline = spannerConfig.getCommitDeadline();
-    if (commitDeadline != null && commitDeadline.get().getMillis() > 0) {
+    // Set retryable codes
+    if (spannerConfig.getRetryableCodes() != null) {

Review comment:
       Makes sense, thanks for trying. I think this approach is nice.

##########
File path: sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/SpannerChangeStreamErrorTest.java
##########
@@ -0,0 +1,494 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.spanner.changestreams;
+
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_CREATED_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_END_TIMESTAMP;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_FINISHED_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_HEARTBEAT_MILLIS;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_PARENT_TOKENS;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_PARTITION_TOKEN;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_RUNNING_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_SCHEDULED_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_START_TIMESTAMP;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_STATE;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_WATERMARK;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.google.api.gax.grpc.testing.MockServiceHelper;
+import com.google.api.gax.retrying.RetrySettings;
+import com.google.cloud.Timestamp;
+import com.google.cloud.spanner.ErrorCode;
+import com.google.cloud.spanner.MockSpannerServiceImpl;
+import com.google.cloud.spanner.MockSpannerServiceImpl.SimulatedExecutionTime;
+import com.google.cloud.spanner.MockSpannerServiceImpl.StatementResult;
+import com.google.cloud.spanner.Statement;
+import com.google.protobuf.ListValue;
+import com.google.protobuf.NullValue;
+import com.google.protobuf.Value;
+import com.google.spanner.v1.ExecuteSqlRequest;
+import com.google.spanner.v1.ResultSet;
+import com.google.spanner.v1.ResultSetMetadata;
+import com.google.spanner.v1.StructType;
+import com.google.spanner.v1.StructType.Field;
+import com.google.spanner.v1.Type;
+import com.google.spanner.v1.TypeCode;
+import io.grpc.Status;
+import java.io.Serializable;
+import java.util.Collections;
+import org.apache.beam.sdk.Pipeline.PipelineExecutionException;
+import org.apache.beam.sdk.io.gcp.spanner.SpannerConfig;
+import org.apache.beam.sdk.io.gcp.spanner.SpannerIO;
+import org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.DaoFactory;
+import org.apache.beam.sdk.io.gcp.spanner.changestreams.model.PartitionMetadata.State;
+import org.apache.beam.sdk.options.ValueProvider.StaticValueProvider;
+import org.apache.beam.sdk.testing.TestPipeline;
+import org.apache.beam.vendor.guava.v26_0_jre.com.google.common.collect.ImmutableSet;
+import org.hamcrest.Matchers;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public class SpannerChangeStreamErrorTest implements Serializable {
+
+  public static final String SPANNER_HOST = "my-host";
+  private static final String TEST_PROJECT = "my-project";
+  private static final String TEST_INSTANCE = "my-instance";
+  private static final String TEST_DATABASE = "my-database";
+  private static final String TEST_TABLE = "my-metadata-table";
+  private static final String TEST_CHANGE_STREAM = "my-change-stream";
+
+  @Rule
+  public final transient TestPipeline pipeline =
+      TestPipeline.create().enableAbandonedNodeEnforcement(false);
+
+  @Rule public final transient ExpectedException thrown = ExpectedException.none();
+
+  private MockSpannerServiceImpl mockSpannerService;
+  private MockServiceHelper serviceHelper;
+
+  @Before
+  public void setUp() throws Exception {
+    mockSpannerService = new MockSpannerServiceImpl();
+    serviceHelper =
+        new MockServiceHelper(SPANNER_HOST, Collections.singletonList(mockSpannerService));
+    serviceHelper.start();
+    serviceHelper.reset();
+  }
+
+  @After
+  public void tearDown() throws NoSuchFieldException, IllegalAccessException {
+    serviceHelper.reset();
+    serviceHelper.stop();
+    mockSpannerService.reset();
+    resetDaoFactoryFields();
+  }
+
+  @Test
+  public void testResourceExhaustedDoesNotRetry() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofStickyException(Status.RESOURCE_EXHAUSTED.asRuntimeException()));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.RESOURCE_EXHAUSTED.name());
+    }
+  }
+
+  @Test
+  public void testUnavailableExceptionRetries() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofExceptions(
+            ImmutableSet.of(
+                Status.UNAVAILABLE.asRuntimeException(),
+                Status.RESOURCE_EXHAUSTED.asRuntimeException())));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      assertThat(
+          mockSpannerService.countRequestsOfType(ExecuteSqlRequest.class), Matchers.greaterThan(1));
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.RESOURCE_EXHAUSTED.name());
+    }
+  }
+
+  @Test
+  public void testAbortedExceptionRetries() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofStickyException(Status.ABORTED.asRuntimeException()));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      assertThat(
+          mockSpannerService.countRequestsOfType(ExecuteSqlRequest.class), Matchers.greaterThan(1));
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.ABORTED.name());
+    }
+  }
+
+  @Test
+  public void testUnknownExceptionDoesNotRetry() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofStickyException(Status.UNKNOWN.asRuntimeException()));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      assertThat(
+          mockSpannerService.countRequestsOfType(ExecuteSqlRequest.class), Matchers.equalTo(1));
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.UNKNOWN.name());
+    }
+  }
+
+  @Test
+  public void testInvalidRecordReceived() {
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+
+    mockTableExists();
+    ResultSet getPartitionResultSet = mockGetParentPartition(now, after3Seconds);
+    mockGetWatermark(now);
+    mockGetPartitionsAfter(
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds(), now.getNanos() + 1000),
+        getPartitionResultSet);
+    mockGetPartitionsAfter(
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds(), now.getNanos() - 1000),
+        getPartitionResultSet);
+
+    Statement changeStreamQueryStatement =
+        Statement.newBuilder(
+                "SELECT * FROM READ_my-change-stream(   start_timestamp => @startTimestamp,   end_timestamp => @endTimestamp,   partition_token => @partitionToken,   read_options => null,   heartbeat_milliseconds => @heartbeatMillis)")
+            .bind("startTimestamp")
+            .to(now)
+            .bind("endTimestamp")
+            .to(after3Seconds)
+            .bind("partitionToken")
+            .to((String) null)
+            .bind("heartbeatMillis")
+            .to(500)
+            .build();
+    ResultSetMetadata readChangeStreamResultSetMetadata =

Review comment:
       nit: could we extract these to methods as well?




-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] zoercai commented on a change in pull request #16846: test: Add sad path tests for Cloud Spanner's ReadChangeStream

Posted by GitBox <gi...@apache.org>.
zoercai commented on a change in pull request #16846:
URL: https://github.com/apache/beam/pull/16846#discussion_r806469295



##########
File path: sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/SpannerChangeStreamErrorTest.java
##########
@@ -0,0 +1,494 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.spanner.changestreams;
+
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_CREATED_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_END_TIMESTAMP;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_FINISHED_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_HEARTBEAT_MILLIS;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_PARENT_TOKENS;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_PARTITION_TOKEN;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_RUNNING_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_SCHEDULED_AT;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_START_TIMESTAMP;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_STATE;
+import static org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.PartitionMetadataAdminDao.COLUMN_WATERMARK;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.google.api.gax.grpc.testing.MockServiceHelper;
+import com.google.api.gax.retrying.RetrySettings;
+import com.google.cloud.Timestamp;
+import com.google.cloud.spanner.ErrorCode;
+import com.google.cloud.spanner.MockSpannerServiceImpl;
+import com.google.cloud.spanner.MockSpannerServiceImpl.SimulatedExecutionTime;
+import com.google.cloud.spanner.MockSpannerServiceImpl.StatementResult;
+import com.google.cloud.spanner.Statement;
+import com.google.protobuf.ListValue;
+import com.google.protobuf.NullValue;
+import com.google.protobuf.Value;
+import com.google.spanner.v1.ExecuteSqlRequest;
+import com.google.spanner.v1.ResultSet;
+import com.google.spanner.v1.ResultSetMetadata;
+import com.google.spanner.v1.StructType;
+import com.google.spanner.v1.StructType.Field;
+import com.google.spanner.v1.Type;
+import com.google.spanner.v1.TypeCode;
+import io.grpc.Status;
+import java.io.Serializable;
+import java.util.Collections;
+import org.apache.beam.sdk.Pipeline.PipelineExecutionException;
+import org.apache.beam.sdk.io.gcp.spanner.SpannerConfig;
+import org.apache.beam.sdk.io.gcp.spanner.SpannerIO;
+import org.apache.beam.sdk.io.gcp.spanner.changestreams.dao.DaoFactory;
+import org.apache.beam.sdk.io.gcp.spanner.changestreams.model.PartitionMetadata.State;
+import org.apache.beam.sdk.options.ValueProvider.StaticValueProvider;
+import org.apache.beam.sdk.testing.TestPipeline;
+import org.apache.beam.vendor.guava.v26_0_jre.com.google.common.collect.ImmutableSet;
+import org.hamcrest.Matchers;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public class SpannerChangeStreamErrorTest implements Serializable {
+
+  public static final String SPANNER_HOST = "my-host";
+  private static final String TEST_PROJECT = "my-project";
+  private static final String TEST_INSTANCE = "my-instance";
+  private static final String TEST_DATABASE = "my-database";
+  private static final String TEST_TABLE = "my-metadata-table";
+  private static final String TEST_CHANGE_STREAM = "my-change-stream";
+
+  @Rule
+  public final transient TestPipeline pipeline =
+      TestPipeline.create().enableAbandonedNodeEnforcement(false);
+
+  @Rule public final transient ExpectedException thrown = ExpectedException.none();
+
+  private MockSpannerServiceImpl mockSpannerService;
+  private MockServiceHelper serviceHelper;
+
+  @Before
+  public void setUp() throws Exception {
+    mockSpannerService = new MockSpannerServiceImpl();
+    serviceHelper =
+        new MockServiceHelper(SPANNER_HOST, Collections.singletonList(mockSpannerService));
+    serviceHelper.start();
+    serviceHelper.reset();
+  }
+
+  @After
+  public void tearDown() throws NoSuchFieldException, IllegalAccessException {
+    serviceHelper.reset();
+    serviceHelper.stop();
+    mockSpannerService.reset();
+    resetDaoFactoryFields();
+  }
+
+  @Test
+  public void testResourceExhaustedDoesNotRetry() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofStickyException(Status.RESOURCE_EXHAUSTED.asRuntimeException()));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.RESOURCE_EXHAUSTED.name());
+    }
+  }
+
+  @Test
+  public void testUnavailableExceptionRetries() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofExceptions(
+            ImmutableSet.of(
+                Status.UNAVAILABLE.asRuntimeException(),
+                Status.RESOURCE_EXHAUSTED.asRuntimeException())));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      assertThat(
+          mockSpannerService.countRequestsOfType(ExecuteSqlRequest.class), Matchers.greaterThan(1));
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.RESOURCE_EXHAUSTED.name());
+    }
+  }
+
+  @Test
+  public void testAbortedExceptionRetries() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofStickyException(Status.ABORTED.asRuntimeException()));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      assertThat(
+          mockSpannerService.countRequestsOfType(ExecuteSqlRequest.class), Matchers.greaterThan(1));
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.ABORTED.name());
+    }
+  }
+
+  @Test
+  public void testUnknownExceptionDoesNotRetry() {
+    mockSpannerService.setExecuteStreamingSqlExecutionTime(
+        SimulatedExecutionTime.ofStickyException(Status.UNKNOWN.asRuntimeException()));
+
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+    try {
+      pipeline.apply(
+          SpannerIO.readChangeStream()
+              .withSpannerConfig(getSpannerConfig())
+              .withChangeStreamName(TEST_CHANGE_STREAM)
+              .withMetadataDatabase(TEST_DATABASE)
+              .withInclusiveStartAt(now)
+              .withInclusiveEndAt(after3Seconds));
+      pipeline.run().waitUntilFinish();
+    } finally {
+      assertThat(
+          mockSpannerService.countRequestsOfType(ExecuteSqlRequest.class), Matchers.equalTo(1));
+      thrown.expect(PipelineExecutionException.class);
+      thrown.expectMessage(ErrorCode.UNKNOWN.name());
+    }
+  }
+
+  @Test
+  public void testInvalidRecordReceived() {
+    final Timestamp now = Timestamp.now();
+    final Timestamp after3Seconds =
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds() + 3, now.getNanos());
+
+    mockTableExists();
+    ResultSet getPartitionResultSet = mockGetParentPartition(now, after3Seconds);
+    mockGetWatermark(now);
+    mockGetPartitionsAfter(
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds(), now.getNanos() + 1000),
+        getPartitionResultSet);
+    mockGetPartitionsAfter(
+        Timestamp.ofTimeSecondsAndNanos(now.getSeconds(), now.getNanos() - 1000),
+        getPartitionResultSet);
+
+    Statement changeStreamQueryStatement =
+        Statement.newBuilder(
+                "SELECT * FROM READ_my-change-stream(   start_timestamp => @startTimestamp,   end_timestamp => @endTimestamp,   partition_token => @partitionToken,   read_options => null,   heartbeat_milliseconds => @heartbeatMillis)")

Review comment:
       This is actually how it looks in the actual request statement, I believe the test will fail if I take out the spaces — https://github.com/googleapis/java-spanner/blob/main/google-cloud-spanner/src/test/java/com/google/cloud/spanner/MockSpannerServiceImpl.java#L113




-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org