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 2020/04/26 16:33:32 UTC
[GitHub] [beam] nielm opened a new pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
nielm opened a new pull request #11529:
URL: https://github.com/apache/beam/pull/11529
Batching is often disabled in streaming pipelines to improve latency. When this is the case, there is no
point in adding several transforms to group/sort/batch the elements, so if batching is disabled, a simple pipeline is used.
Post-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
Lang | SDK | Apex | Dataflow | Flink | Gearpump | Samza | Spark
--- | --- | --- | --- | --- | --- | --- | ---
Go | [](https://builds.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
Java | [](https://builds.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Apex/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Gearpump/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
Python | [](https://builds.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/) | --- | [](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/)
XLang | --- | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/)
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
--- |Java | Python | Go | Website
--- | --- | --- | --- | ---
Non-portable | [](https://builds.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/)
Portable | --- | [](https://builds.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | ---
See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] nielm commented on pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
Posted by GitBox <gi...@apache.org>.
nielm commented on pull request #11529:
URL: https://github.com/apache/beam/pull/11529#issuecomment-630753062
Retest this please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] nielm removed a comment on pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
Posted by GitBox <gi...@apache.org>.
nielm removed a comment on pull request #11529:
URL: https://github.com/apache/beam/pull/11529#issuecomment-630476722
Retest this please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] nielm removed a comment on pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
Posted by GitBox <gi...@apache.org>.
nielm removed a comment on pull request #11529:
URL: https://github.com/apache/beam/pull/11529#issuecomment-620007939
retest this please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on pull request #11529:
URL: https://github.com/apache/beam/pull/11529#issuecomment-630495233
Retest this please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on pull request #11529:
URL: https://github.com/apache/beam/pull/11529#issuecomment-631668950
Retest this please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on a change in pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on a change in pull request #11529:
URL: https://github.com/apache/beam/pull/11529#discussion_r428237904
##########
File path: sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIOWriteTest.java
##########
@@ -263,6 +263,17 @@ private void verifyBatches(Iterable<Mutation>... batches) {
@Test
public void noBatching() throws Exception {
+
+ // This test uses a different mock/fake because it explicitly does not want to populate the
+ // Spanner schema.
+ FakeServiceFactory fakeServiceFactory = new FakeServiceFactory();
+ ReadOnlyTransaction tx = mock(ReadOnlyTransaction.class);
+ when(fakeServiceFactory.mockDatabaseClient().readOnlyTransaction()).thenReturn(tx);
+
+ // Capture batches sent to writeAtLeastOnce.
+ when(fakeServiceFactory.mockDatabaseClient().writeAtLeastOnce(mutationBatchesCaptor.capture()))
+ .thenReturn(null);
+
Review comment:
Ah I see. But should you be using `fakeServiceFactory` throughout this test then? There are still references to `serviceFactory`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on pull request #11529:
URL: https://github.com/apache/beam/pull/11529#issuecomment-631680948
Retest this please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] nielm commented on pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
Posted by GitBox <gi...@apache.org>.
nielm commented on pull request #11529:
URL: https://github.com/apache/beam/pull/11529#issuecomment-630476722
Retest this please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on pull request #11529:
URL: https://github.com/apache/beam/pull/11529#issuecomment-631681140
Jenkins seems to be very sleepy today...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] udim commented on pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
Posted by GitBox <gi...@apache.org>.
udim commented on pull request #11529:
URL: https://github.com/apache/beam/pull/11529#issuecomment-631692370
Retest this please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on pull request #11529:
URL: https://github.com/apache/beam/pull/11529#issuecomment-631689149
Retest this please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on pull request #11529:
URL: https://github.com/apache/beam/pull/11529#issuecomment-631692332
retest this please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] nielm removed a comment on pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
Posted by GitBox <gi...@apache.org>.
nielm removed a comment on pull request #11529:
URL: https://github.com/apache/beam/pull/11529#issuecomment-630491891
Retest this please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on a change in pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on a change in pull request #11529:
URL: https://github.com/apache/beam/pull/11529#discussion_r427580795
##########
File path: sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIOWriteTest.java
##########
@@ -263,6 +263,17 @@ private void verifyBatches(Iterable<Mutation>... batches) {
@Test
public void noBatching() throws Exception {
+
+ // This test uses a different mock/fake because it explicitly does not want to populate the
+ // Spanner schema.
+ FakeServiceFactory fakeServiceFactory = new FakeServiceFactory();
+ ReadOnlyTransaction tx = mock(ReadOnlyTransaction.class);
+ when(fakeServiceFactory.mockDatabaseClient().readOnlyTransaction()).thenReturn(tx);
+
+ // Capture batches sent to writeAtLeastOnce.
+ when(fakeServiceFactory.mockDatabaseClient().writeAtLeastOnce(mutationBatchesCaptor.capture()))
+ .thenReturn(null);
+
Review comment:
Actually on second thought I wonder if we should just leave this test completely unchanged (i.e. call `verifyBatches`, don't check that the DB schema is never read). The fact the schema isn't read seems like an implementation detail that we don't need to explicitly verify. What do you think?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on pull request #11529:
URL: https://github.com/apache/beam/pull/11529#issuecomment-631055788
Retest this please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on pull request #11529:
URL: https://github.com/apache/beam/pull/11529#issuecomment-631794665
Run Java PreCommit
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] nielm commented on pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
Posted by GitBox <gi...@apache.org>.
nielm commented on pull request #11529:
URL: https://github.com/apache/beam/pull/11529#issuecomment-631668908
Retest this please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on pull request #11529:
URL: https://github.com/apache/beam/pull/11529#issuecomment-631669226
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on pull request #11529:
URL: https://github.com/apache/beam/pull/11529#issuecomment-631792217
Run Java PostCommit
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on pull request #11529:
URL: https://github.com/apache/beam/pull/11529#issuecomment-631670617
Retest this please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] nielm commented on pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
Posted by GitBox <gi...@apache.org>.
nielm commented on pull request #11529:
URL: https://github.com/apache/beam/pull/11529#issuecomment-620007939
retest this please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on a change in pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on a change in pull request #11529:
URL: https://github.com/apache/beam/pull/11529#discussion_r427576985
##########
File path: sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIOWriteTest.java
##########
@@ -263,6 +263,17 @@ private void verifyBatches(Iterable<Mutation>... batches) {
@Test
public void noBatching() throws Exception {
+
+ // This test uses a different mock/fake because it explicitly does not want to populate the
+ // Spanner schema.
+ FakeServiceFactory fakeServiceFactory = new FakeServiceFactory();
+ ReadOnlyTransaction tx = mock(ReadOnlyTransaction.class);
+ when(fakeServiceFactory.mockDatabaseClient().readOnlyTransaction()).thenReturn(tx);
+
+ // Capture batches sent to writeAtLeastOnce.
+ when(fakeServiceFactory.mockDatabaseClient().writeAtLeastOnce(mutationBatchesCaptor.capture()))
+ .thenReturn(null);
+
Review comment:
This doesn't seem to be used. It seems we should either:
- Pass `fakeServiceFactory` to `withServiceFactory` and the verify calls below, or
- Just remove `fakeServiceFactory` and use `serviceFactory` here instead. It seems to be capable of verifying that the schema isn't read, so I'm not sure there's value in building another fake that doesn't have a schema.
I'd prefer the latter, but I'd be fine with the former if you think that's better.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on pull request #11529:
URL: https://github.com/apache/beam/pull/11529#issuecomment-631747496
Retest this please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on pull request #11529:
URL: https://github.com/apache/beam/pull/11529#issuecomment-631789673
Run Java PostCommit
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on pull request #11529:
URL: https://github.com/apache/beam/pull/11529#issuecomment-631792367
Run Java PreCommit
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] nielm commented on a change in pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
Posted by GitBox <gi...@apache.org>.
nielm commented on a change in pull request #11529:
URL: https://github.com/apache/beam/pull/11529#discussion_r428235563
##########
File path: sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIOWriteTest.java
##########
@@ -263,6 +263,17 @@ private void verifyBatches(Iterable<Mutation>... batches) {
@Test
public void noBatching() throws Exception {
+
+ // This test uses a different mock/fake because it explicitly does not want to populate the
+ // Spanner schema.
+ FakeServiceFactory fakeServiceFactory = new FakeServiceFactory();
+ ReadOnlyTransaction tx = mock(ReadOnlyTransaction.class);
+ when(fakeServiceFactory.mockDatabaseClient().readOnlyTransaction()).thenReturn(tx);
+
+ // Capture batches sent to writeAtLeastOnce.
+ when(fakeServiceFactory.mockDatabaseClient().writeAtLeastOnce(mutationBatchesCaptor.capture()))
+ .thenReturn(null);
+
Review comment:
This test specifically tests that the pipeline stage for reading the schema (required for grouping/batching) are not being used, verifying that the batching is being bypassed. Yes, this is an implementation detail, but I added this part of the test so that future changes do not break this behaviour.
I cannot use the already-created serviceFactory here because that does not give me access to the mock ReadOnlyTransaction that I need to verify that executeQuery() is never called.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit merged pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
Posted by GitBox <gi...@apache.org>.
TheNeuralBit merged pull request #11529:
URL: https://github.com/apache/beam/pull/11529
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on pull request #11529:
URL: https://github.com/apache/beam/pull/11529#issuecomment-631676277
Retest this please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] nielm commented on a change in pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
Posted by GitBox <gi...@apache.org>.
nielm commented on a change in pull request #11529:
URL: https://github.com/apache/beam/pull/11529#discussion_r428246820
##########
File path: sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIOWriteTest.java
##########
@@ -263,6 +263,17 @@ private void verifyBatches(Iterable<Mutation>... batches) {
@Test
public void noBatching() throws Exception {
+
+ // This test uses a different mock/fake because it explicitly does not want to populate the
+ // Spanner schema.
+ FakeServiceFactory fakeServiceFactory = new FakeServiceFactory();
+ ReadOnlyTransaction tx = mock(ReadOnlyTransaction.class);
+ when(fakeServiceFactory.mockDatabaseClient().readOnlyTransaction()).thenReturn(tx);
+
+ // Capture batches sent to writeAtLeastOnce.
+ when(fakeServiceFactory.mockDatabaseClient().writeAtLeastOnce(mutationBatchesCaptor.capture()))
+ .thenReturn(null);
+
Review comment:
Oops, yes, correct. Fixed now.
> Ah I see. But should you be using `fakeServiceFactory` throughout this test then? There are still references to `serviceFactory`
Oops, yes, correct! Fixed 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] nielm commented on pull request #11529: [BEAM-9822] Simplify pipeline when batching is disabled.
Posted by GitBox <gi...@apache.org>.
nielm commented on pull request #11529:
URL: https://github.com/apache/beam/pull/11529#issuecomment-630491891
Retest this please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org