[GitHub] [beam] y1chi commented on pull request #12191: [BEAM-10419] Skip FhirIORead integration test due to flakiness

2020-07-07 Thread GitBox


y1chi commented on pull request #12191:
URL: https://github.com/apache/beam/pull/12191#issuecomment-655296773


   R: @aaltay 
   CC: @jaketf 



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:
us...@infra.apache.org




[GitHub] [beam] pabloem commented on pull request #12151: [BEAM-9896] Add streaming for SnowflakeIO.Write to Java SDK

2020-07-07 Thread GitBox


pabloem commented on pull request #12151:
URL: https://github.com/apache/beam/pull/12151#issuecomment-655286584


   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:
us...@infra.apache.org




[GitHub] [beam] pabloem commented on pull request #12151: [BEAM-9896] Add streaming for SnowflakeIO.Write to Java SDK

2020-07-07 Thread GitBox


pabloem commented on pull request #12151:
URL: https://github.com/apache/beam/pull/12151#issuecomment-655286374


   sorry! Yes, I'll take a look..



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:
us...@infra.apache.org




[GitHub] [beam] tvalentyn commented on pull request #12194: [DO NOT MERGE] Run all PostCommit and PreCommit Tests against Release Branch

2020-07-07 Thread GitBox


tvalentyn commented on pull request #12194:
URL: https://github.com/apache/beam/pull/12194#issuecomment-655260089


   Run Samza ValidatesRunner



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:
us...@infra.apache.org




[GitHub] [beam] tvalentyn commented on pull request #12194: [DO NOT MERGE] Run all PostCommit and PreCommit Tests against Release Branch

2020-07-07 Thread GitBox


tvalentyn commented on pull request #12194:
URL: https://github.com/apache/beam/pull/12194#issuecomment-655259903


   Run Python2_PVR_Flink 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:
us...@infra.apache.org




[GitHub] [beam] tvalentyn commented on pull request #12194: [DO NOT MERGE] Run all PostCommit and PreCommit Tests against Release Branch

2020-07-07 Thread GitBox


tvalentyn commented on pull request #12194:
URL: https://github.com/apache/beam/pull/12194#issuecomment-655257992


   Run Java_Examples_Dataflow 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:
us...@infra.apache.org




[GitHub] [beam] lukecwik edited a comment on pull request #12192: [WIP][BEAM-10420] Migrate SDF logic into PerWindowInvoker

2020-07-07 Thread GitBox


lukecwik edited a comment on pull request #12192:
URL: https://github.com/apache/beam/pull/12192#issuecomment-655239038


   R: @robertwb @chamikaramj 
   
   Progress reporting needs to be able to see which window out of how many are 
being processed. Splitting needs to be able to see which window is being 
processed to compute the primary and residual correctly.
   
   Alternatively, I could move the window observing logic outside of the 
DoFnInvoker and then all the DoFnRunners would own this logic (this does seem 
like it would lead to more code duplication).
   
   WDYT?



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:
us...@infra.apache.org




[GitHub] [beam] tvalentyn commented on pull request #12194: [DO NOT MERGE] Run all PostCommit and PreCommit Tests against Release Branch

2020-07-07 Thread GitBox


tvalentyn commented on pull request #12194:
URL: https://github.com/apache/beam/pull/12194#issuecomment-655248020







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:
us...@infra.apache.org




[GitHub] [beam] lukecwik closed pull request #12192: [WIP][BEAM-10420] Migrate SDF logic into PerWindowInvoker

2020-07-07 Thread GitBox


lukecwik closed pull request #12192:
URL: https://github.com/apache/beam/pull/12192


   



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:
us...@infra.apache.org




[GitHub] [beam] tvalentyn opened a new pull request #12194: [DO NOT MERGE] Run all PostCommit and PreCommit Tests against Release Branch

2020-07-07 Thread GitBox


tvalentyn opened a new pull request #12194:
URL: https://github.com/apache/beam/pull/12194


   You can run many tests automatically using 
release/src/main/scripts/mass_comment.py.



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:
us...@infra.apache.org




[GitHub] [beam] bamnet commented on pull request #12193: [BEAM-8472] get default GCP region option (Go)

2020-07-07 Thread GitBox


bamnet commented on pull request #12193:
URL: https://github.com/apache/beam/pull/12193#issuecomment-655241403


   R: @lostluck
   
   /fyi @ibzib



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:
us...@infra.apache.org




[GitHub] [beam] lukecwik edited a comment on pull request #12192: [WIP][BEAM-10420] Migrate SDF logic into PerWindowInvoker

2020-07-07 Thread GitBox


lukecwik edited a comment on pull request #12192:
URL: https://github.com/apache/beam/pull/12192#issuecomment-655239038


   R: @robertwb @chamikaramj 
   
   Progress reporting needs to be able to see which window out of how many are 
being processed. Splitting needs to be able to see which window is being 
processed to compute the primary and residual correctly.
   
   Style-wise does it make sense to use the `kwargs` to pass these through or 
should I continue to make them explicit like the `restriction_tracker` and 
`watermark_estimator` that was provided before.
   
   Alternatively, I could move the window observing logic outside of the 
DoFnInvoker and then all the DoFnRunners would own this logic (this does seem 
like it would lead to more code duplication).
   
   WDYT?



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:
us...@infra.apache.org




[GitHub] [beam] danielxjd commented on a change in pull request #12106: [BEAM-9792] Fixing the IOException handling in InsertAll for BigQuery

2020-07-07 Thread GitBox


danielxjd commented on a change in pull request #12106:
URL: https://github.com/apache/beam/pull/12106#discussion_r451244400



##
File path: 
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryServicesImpl.java
##
@@ -893,6 +906,19 @@ public void deleteDataset(String projectId, String 
datasetId)
   ignoreInsertIds);
 }
 
+protected GoogleJsonError.ErrorInfo getErrorInfo(IOException e) {
+  GoogleJsonResponseException jsonCause = null;

Review comment:
   True, I have further simplified the method.





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:
us...@infra.apache.org




[GitHub] [beam] bamnet opened a new pull request #12193: [BEAM-8472] get default GCP region option (Go)

2020-07-07 Thread GitBox


bamnet opened a new pull request #12193:
URL: https://github.com/apache/beam/pull/12193


   If the region flag is unset, attempt to extract a region from 
$CLOUDSDK_COMPUTE_REGION and the gcloud SDK.
   
   This mirrors the 
[Java](https://github.com/apache/beam/blob/64e69d68f85903d94170fa2efc5a1633d82da589/runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/options/DataflowPipelineOptions.java#L218)
 and 
[Python](https://github.com/apache/beam/blob/72d9744affc6ff4a63816f737e18c4abe14f245f/sdks/python/apache_beam/runners/dataflow/dataflow_runner.py#L1437)
 implementation.
   
   
   
   Thank you for your contribution! Follow this checklist to help us 
incorporate your contribution quickly and easily:
   
- [X] [**Choose 
reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and 
mention them in a comment (`R: @username`).
- [X] Format the pull request title like `[BEAM-XXX] Fixes bug in 
ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA 
issue, if applicable. This will automatically link the pull request to the 
issue.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual 
Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   See the [Contributor Guide](https://beam.apache.org/contribute) for more 
tips on [how to make review process 
smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
   
   Post-Commit Tests Status (on master branch)
   

   
   Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
   --- | --- | --- | --- | --- | --- | ---
   Go | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/)
 | --- | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/)
 | --- | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
 | ---
   Java | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
 | [![Build 

[GitHub] [beam] lukecwik commented on a change in pull request #12192: [WIP][BEAM-10420] Migrate SDF logic into PerWindowInvoker

2020-07-07 Thread GitBox


lukecwik commented on a change in pull request #12192:
URL: https://github.com/apache/beam/pull/12192#discussion_r451243088



##
File path: sdks/python/apache_beam/runners/direct/sdf_direct_runner.py
##
@@ -464,19 +461,15 @@ def initiate_checkpoint():
   with self._checkpoint_lock:
 if checkpoint_state.checkpointed:
   return
-  checkpoint_state.residual_restriction = tracker.checkpoint()
-  checkpoint_state.checkpointed = object()
+checkpoint_state.checkpointed = object()

Review comment:
   This is a bug fix, we should be setting checkpoint_state.checkpointed 
while holding the lock.





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:
us...@infra.apache.org




[GitHub] [beam] lukecwik commented on a change in pull request #12192: [WIP][BEAM-10420] Migrate SDF logic into PerWindowInvoker

2020-07-07 Thread GitBox


lukecwik commented on a change in pull request #12192:
URL: https://github.com/apache/beam/pull/12192#discussion_r451242979



##
File path: sdks/python/apache_beam/runners/direct/sdf_direct_runner.py
##
@@ -507,7 +500,6 @@ def initiate_checkpoint():
   if self._max_num_outputs and output_count >= self._max_num_outputs:
 initiate_checkpoint()
 
-tracker.check_done()

Review comment:
   This is handled within the PerWindowInvoker already.





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:
us...@infra.apache.org




[GitHub] [beam] lukecwik edited a comment on pull request #12192: [WIP][BEAM-10420] Migrate SDF logic into PerWindowInvoker

2020-07-07 Thread GitBox


lukecwik edited a comment on pull request #12192:
URL: https://github.com/apache/beam/pull/12192#issuecomment-655239038


   R: @robertwb @chamikaramj 
   
   Style-wise does it make sense to use the `kwargs` to pass these through or 
should I continue to make them explicit like the `restriction_tracker` and 
`watermark_estimator` that was provided before.
   
   Alternatively, I could move the window observing logic outside of the 
DoFnInvoker and then all the DoFnRunners would own this logic (this does seem 
like it would lead to more code duplication).
   
   WDYT?



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:
us...@infra.apache.org




[GitHub] [beam] lukecwik commented on pull request #12192: [WIP][BEAM-10420] Migrate SDF logic into PerWindowInvoker

2020-07-07 Thread GitBox


lukecwik commented on pull request #12192:
URL: https://github.com/apache/beam/pull/12192#issuecomment-655239038


   R: @robertwb @chamikaramj 
   
   Style-wise does it make sense to use the `kwargs` to pass these through or 
should I continue to make them explicit like the `restriction_tracker` and 
`watermark_estimator` that was provided before.
   
   Alternatively, I could move the window observing logic outside of the 
DoFnInvoker and then all the DoFnRunners would own this logic.
   
   WDYT?



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:
us...@infra.apache.org




[GitHub] [beam] chamikaramj commented on a change in pull request #12106: [BEAM-9792] Fixing the IOException handling in InsertAll for BigQuery

2020-07-07 Thread GitBox


chamikaramj commented on a change in pull request #12106:
URL: https://github.com/apache/beam/pull/12106#discussion_r451242654



##
File path: 
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryServicesImpl.java
##
@@ -893,6 +906,19 @@ public void deleteDataset(String projectId, String 
datasetId)
   ignoreInsertIds);
 }
 
+protected GoogleJsonError.ErrorInfo getErrorInfo(IOException e) {
+  GoogleJsonResponseException jsonCause = null;

Review comment:
   We can further simplify :)
   protected GoogleJsonError.ErrorInfo getErrorInfo(IOException e) {
   if (!(e instanceof GoogleJsonResponseException)) {
   return null;
   }
   GoogleJsonError jsonError = ((GoogleJsonResponseException) 
e).getDetails();
   ...





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:
us...@infra.apache.org




[GitHub] [beam] lukecwik opened a new pull request #12192: [BEAM-10420] Migrate SDF logic into PerWindowInvoker

2020-07-07 Thread GitBox


lukecwik opened a new pull request #12192:
URL: https://github.com/apache/beam/pull/12192


   To be able to have the windowing optimization stay within the 
PerWindowInvoker, I needed to have it control the creation of the watermark 
estimator and the restriction tracker thus I supply the restrication and 
watermark estimator state.
   
   
   
   Thank you for your contribution! Follow this checklist to help us 
incorporate your contribution quickly and easily:
   
- [ ] [**Choose 
reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and 
mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in 
ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA 
issue, if applicable. This will automatically link the pull request to the 
issue.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual 
Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   See the [Contributor Guide](https://beam.apache.org/contribute) for more 
tips on [how to make review process 
smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
   
   Post-Commit Tests Status (on master branch)
   

   
   Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
   --- | --- | --- | --- | --- | --- | ---
   Go | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/)
 | --- | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/)
 | --- | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
 | ---
   Java | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
 | [![Build 
Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
   Python | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)[![Build
 

[GitHub] [beam] danielxjd commented on a change in pull request #12106: [BEAM-9792] Fixing the IOException handling in InsertAll for BigQuery

2020-07-07 Thread GitBox


danielxjd commented on a change in pull request #12106:
URL: https://github.com/apache/beam/pull/12106#discussion_r451241677



##
File path: 
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryServicesImpl.java
##
@@ -893,6 +903,20 @@ public void deleteDataset(String projectId, String 
datasetId)
   ignoreInsertIds);
 }
 
+private GoogleJsonError.ErrorInfo getErrorInfo(IOException e) {
+  GoogleJsonResponseException jsonCause = null;
+  Throwable eCause = e;
+  if (eCause instanceof GoogleJsonResponseException) {

Review comment:
   Yup, that should work 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:
us...@infra.apache.org




[GitHub] [beam] udim commented on a change in pull request #12009: [BEAM-10258] Support type hint annotations on PTransform's expand()

2020-07-07 Thread GitBox


udim commented on a change in pull request #12009:
URL: https://github.com/apache/beam/pull/12009#discussion_r451233193



##
File path: sdks/python/apache_beam/typehints/decorators.py
##
@@ -378,6 +378,56 @@ def has_simple_output_type(self):
 self.output_types and len(self.output_types[0]) == 1 and
 not self.output_types[1])
 
+  def strip_pcoll(self):
+return self.strip_pcoll_helper(self.input_types,
+   self._has_input_types,
+   {'input_types': None},
+   ['apache_beam.pvalue.PBegin'],
+   'An input typehint to a PTransform must be'
+   ' a single (or nested) type wrapped by '
+   'a PCollection or PBegin. ',
+   'strip_pcoll_input()').\
+strip_pcoll_helper(self.output_types,
+   self.has_simple_output_type,
+   {'output_types': None},
+   ['apache_beam.pvalue.PDone'],
+   'An output typehint to a PTransform must be'
+   ' a single (or nested) type wrapped by '
+   'a PCollection or PDone. ',
+   'strip_pcoll_output()')
+
+  def strip_pcoll_helper(
+  self,
+  my_type,# type: any
+  has_my_type,# type: Callable[[], bool]
+  kwarg_dict, # type: Dict[str, any]
+  my_valid_classes,   # type: List[str]
+  error_str,  # type: str
+  source_str  # type: str
+  ):
+# type: (...) -> IOTypeHints
+
+if not has_my_type() or len(my_type[0]) != 1:
+  return self
+
+my_type = my_type[0][0]
+
+if isinstance(my_type, typehints.AnyTypeConstraint):
+  return self
+
+valid_classes = ['apache_beam.pvalue.PCollection'] + my_valid_classes
+
+if not any(valid_class in str(my_type) for valid_class in valid_classes):

Review comment:
   Are string representations necessary here? We typically don't compare by 
string.
   
   You can also write `if not isinstance(my_type, valid_classes)`.

##
File path: sdks/python/apache_beam/typehints/decorators.py
##
@@ -378,6 +379,61 @@ def has_simple_output_type(self):
 self.output_types and len(self.output_types[0]) == 1 and
 not self.output_types[1])
 
+  def strip_pcoll_input(self):
+# type: () -> IOTypeHints
+
+error_str = 'An input typehint to a PTransform must be a single (or 
nested) type wrapped by a PCollection or ' \
+'PBegin. '
+
+if any(element is None for element in [self.input_types, 
self.input_types[0], self.input_types[0][0]]):

Review comment:
   It does support short-circuiting, but the list `[self.input_types, 
self.input_types[0], self.input_types[0][0]]` is first completely evaluated.

##
File path: website/www/site/content/en/documentation/sdks/python-type-safety.md
##
@@ -90,6 +91,15 @@ The following code declares an `int` input and a `str` 
output type hint on the `
 {{< code_sample 
"sdks/python/apache_beam/examples/snippets/snippets_test_py3.py" 
type_hints_map_annotations >}}
 {{< /highlight >}}
 
+The following code demonstrates how to use annotations on `PTransform` 
subclasses. 
+A valid annotation is a `PCollection`, `PBegin`, or `PDone` that wraps an 
internal (nested) type. 

Review comment:
   @robertwb are PBegin and PDone part of the public API?

##
File path: sdks/python/apache_beam/typehints/decorators.py
##
@@ -378,6 +378,56 @@ def has_simple_output_type(self):
 self.output_types and len(self.output_types[0]) == 1 and
 not self.output_types[1])
 
+  def strip_pcoll(self):
+return self.strip_pcoll_helper(self.input_types,
+   self._has_input_types,
+   {'input_types': None},
+   ['apache_beam.pvalue.PBegin'],
+   'An input typehint to a PTransform must be'
+   ' a single (or nested) type wrapped by '
+   'a PCollection or PBegin. ',
+   'strip_pcoll_input()').\
+strip_pcoll_helper(self.output_types,
+   self.has_simple_output_type,
+   {'output_types': None},
+   ['apache_beam.pvalue.PDone'],
+   'An output typehint to a PTransform must be'
+   ' a single (or nested) type wrapped by '
+   'a PCollection or PDone. ',
+   'strip_pcoll_output()')
+
+  def strip_pcoll_helper(
+

[GitHub] [beam] chamikaramj commented on a change in pull request #12106: [BEAM-9792] Fixing the IOException handling in InsertAll for BigQuery

2020-07-07 Thread GitBox


chamikaramj commented on a change in pull request #12106:
URL: https://github.com/apache/beam/pull/12106#discussion_r451237362



##
File path: 
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryServicesImpl.java
##
@@ -893,6 +903,20 @@ public void deleteDataset(String projectId, String 
datasetId)
   ignoreInsertIds);
 }
 
+private GoogleJsonError.ErrorInfo getErrorInfo(IOException e) {
+  GoogleJsonResponseException jsonCause = null;
+  Throwable eCause = e;
+  if (eCause instanceof GoogleJsonResponseException) {

Review comment:
   Thanks. In that case let's simplify this to following.
   
   if (!(e instanceof GoogleJsonResponseException)) {
 return null;
   }
   
   GoogleJsonResponseException jsonCause = (GoogleJsonResponseException) e;
   ...
   
   





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:
us...@infra.apache.org




[GitHub] [beam] y1chi opened a new pull request #12191: [BEAM-10419] Skip FhirIORead integration test due to flakiness

2020-07-07 Thread GitBox


y1chi opened a new pull request #12191:
URL: https://github.com/apache/beam/pull/12191


   **Please** add a meaningful description for your change here
   
   
   
   Thank you for your contribution! Follow this checklist to help us 
incorporate your contribution quickly and easily:
   
- [ ] [**Choose 
reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and 
mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in 
ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA 
issue, if applicable. This will automatically link the pull request to the 
issue.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual 
Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   See the [Contributor Guide](https://beam.apache.org/contribute) for more 
tips on [how to make review process 
smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
   
   Post-Commit Tests Status (on master branch)
   

   
   Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
   --- | --- | --- | --- | --- | --- | ---
   Go | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/)
 | --- | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/)
 | --- | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
 | ---
   Java | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
 | [![Build 
Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
   Python | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)[![Build
 

[GitHub] [beam] jaketf commented on pull request #11959: refactor HCLS IO ITs to support stores in other projects

2020-07-07 Thread GitBox


jaketf commented on pull request #11959:
URL: https://github.com/apache/beam/pull/11959#issuecomment-655219973


   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:
us...@infra.apache.org




[GitHub] [beam] robertwb commented on pull request #11963: Add relational GroupBy transform to Python.

2020-07-07 Thread GitBox


robertwb commented on pull request #11963:
URL: https://github.com/apache/beam/pull/11963#issuecomment-655215071


   R: @TheNeuralBit This is ready for review.



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:
us...@infra.apache.org




[GitHub] [beam] aaltay commented on pull request #11744: [BEAM-10007] Handle ValueProvider pipeline options in PortableRunner

2020-07-07 Thread GitBox


aaltay commented on pull request #11744:
URL: https://github.com/apache/beam/pull/11744#issuecomment-655211272


   > > Oh I see now. I tried to fix this issue by adding `drop_defaults=true` 
but we explicitly don't drop the default value for ValueProvider arguments:
   > > 
https://github.com/apache/beam/blob/1fbc55e7819187018c2cb3cb295eb84e6f348d8a/sdks/python/apache_beam/options/pipeline_options.py#L319-L320
   > > 
   > > I'm not really sure why that is but it's been there for three years so 
I'm a little hesitant to remove it... I just pushed 
[982e087](https://github.com/apache/beam/commit/982e087fd09509277057b76991598cad3e6acf8e)
 that does this, let's see if it breaks anything in CI or Google tests.
   > > I'll check back in the morning, if it breaks anything (or if anyone 
tells me this is a bad idea), I can re-write the test to only assert on 
specific options.
   > 
   > I do not remember the reason for this. I am looking.
   > 
   > @tvalentyn - do you remember?
   
   I _believe_ default value of RuntimeValueProviders are actually user 
provided non-default values. (If I am reading this correctly: 
https://github.com/apache/beam/blob/1fbc55e7819187018c2cb3cb295eb84e6f348d8a/sdks/python/apache_beam/options/pipeline_options.py#L115)



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:
us...@infra.apache.org




[GitHub] [beam] aaltay commented on pull request #11744: [BEAM-10007] Handle ValueProvider pipeline options in PortableRunner

2020-07-07 Thread GitBox


aaltay commented on pull request #11744:
URL: https://github.com/apache/beam/pull/11744#issuecomment-655209912


   > Oh I see now. I tried to fix this issue by adding `drop_defaults=true` but 
we explicitly don't drop the default value for ValueProvider arguments:
   > 
   > 
https://github.com/apache/beam/blob/1fbc55e7819187018c2cb3cb295eb84e6f348d8a/sdks/python/apache_beam/options/pipeline_options.py#L319-L320
   > 
   > I'm not really sure why that is but it's been there for three years so I'm 
a little hesitant to remove it... I just pushed 
[982e087](https://github.com/apache/beam/commit/982e087fd09509277057b76991598cad3e6acf8e)
 that does this, let's see if it breaks anything in CI or Google tests.
   > 
   > I'll check back in the morning, if it breaks anything (or if anyone tells 
me this is a bad idea), I can re-write the test to only assert on specific 
options.
   
   I do not remember the reason for this. I am looking.
   
   @tvalentyn - do you remember?



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:
us...@infra.apache.org




[GitHub] [beam] TheNeuralBit commented on pull request #11744: [BEAM-10007] Handle ValueProvider pipeline options in PortableRunner

2020-07-07 Thread GitBox


TheNeuralBit commented on pull request #11744:
URL: https://github.com/apache/beam/pull/11744#issuecomment-655207778


   Oh I see now. I tried to fix this issue by adding `drop_defaults=true` but 
we explicitly don't drop the default value for ValueProvider arguments: 
https://github.com/apache/beam/blob/1fbc55e7819187018c2cb3cb295eb84e6f348d8a/sdks/python/apache_beam/options/pipeline_options.py#L319-L320
   
   I'm not really sure why that is but it's been there for three years so I'm a 
little hesitant to remove it... I just pushed 982e087 that does this, let's see 
if it breaks anything in CI or Google tests.
   
   I'll check back in the morning, if it breaks anything (or if anyone tells me 
this is a bad idea), I can re-write the test to only assert on specific options.



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:
us...@infra.apache.org




[GitHub] [beam] danielxjd commented on a change in pull request #12106: [BEAM-9792] Fixing the IOException handling in InsertAll for BigQuery

2020-07-07 Thread GitBox


danielxjd commented on a change in pull request #12106:
URL: https://github.com/apache/beam/pull/12106#discussion_r451213408



##
File path: 
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryServicesImpl.java
##
@@ -893,6 +903,20 @@ public void deleteDataset(String projectId, String 
datasetId)
   ignoreInsertIds);
 }
 
+private GoogleJsonError.ErrorInfo getErrorInfo(IOException e) {
+  GoogleJsonResponseException jsonCause = null;
+  Throwable eCause = e;
+  if (eCause instanceof GoogleJsonResponseException) {

Review comment:
   GoogleJsonResponseException extends HttpResponseException and 
HttpResponseException extends IOException





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:
us...@infra.apache.org




[GitHub] [beam] lukecwik commented on pull request #12184: [BEAM-10392] Make sure that messages that are unroutable are returned to the sender and redelivered.

2020-07-07 Thread GitBox


lukecwik commented on pull request #12184:
URL: https://github.com/apache/beam/pull/12184#issuecomment-655203055


   LGTM?



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:
us...@infra.apache.org




[GitHub] [beam] jaketf commented on pull request #11959: refactor HCLS IO ITs to support stores in other projects

2020-07-07 Thread GitBox


jaketf commented on pull request #11959:
URL: https://github.com/apache/beam/pull/11959#issuecomment-655202173


   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:
us...@infra.apache.org




[GitHub] [beam] KevinGG commented on pull request #12107: Interactive Environment Inspector for messaging

2020-07-07 Thread GitBox


KevinGG commented on pull request #12107:
URL: https://github.com/apache/beam/pull/12107#issuecomment-655194772


   > left just two comments. Please respond (address if you think it's worth 
addressing) - and we can merge
   
   Thanks, @pabloem ! I've made corresponding changes.



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:
us...@infra.apache.org




[GitHub] [beam] TheNeuralBit commented on pull request #11744: [BEAM-10007] Handle ValueProvider pipeline options in PortableRunner

2020-07-07 Thread GitBox


TheNeuralBit commented on pull request #11744:
URL: https://github.com/apache/beam/pull/11744#issuecomment-655192320


   Looks like the CI failure is actually another flake due to BEAM-10006. It 
can be replicated reliably locally with
   
   ```
   $ pytest 
apache_beam/runners/portability/fn_api_runner/fn_runner_test.py::FnApiRunnerTest::test_create_value_provider_pipeline_option
 apache_beam/runners/worker/sdk_worker_main_test.py
   ```



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:
us...@infra.apache.org




[GitHub] [beam] aaltay commented on pull request #11744: [BEAM-10007] Handle ValueProvider pipeline options in PortableRunner

2020-07-07 Thread GitBox


aaltay commented on pull request #11744:
URL: https://github.com/apache/beam/pull/11744#issuecomment-655189985







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:
us...@infra.apache.org




[GitHub] [beam] robertwb merged pull request #12182: [BEAM-4605] Remove portable runner GBK hack.

2020-07-07 Thread GitBox


robertwb merged pull request #12182:
URL: https://github.com/apache/beam/pull/12182


   



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:
us...@infra.apache.org




[GitHub] [beam] aaltay commented on pull request #12128: [BEAM-10417] - Move Shared object from tfx_bsl

2020-07-07 Thread GitBox


aaltay commented on pull request #12128:
URL: https://github.com/apache/beam/pull/12128#issuecomment-655188903


   Run Python 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:
us...@infra.apache.org




[GitHub] [beam] chamikaramj commented on a change in pull request #12106: [BEAM-9792] Fixing the IOException handling in InsertAll for BigQuery

2020-07-07 Thread GitBox


chamikaramj commented on a change in pull request #12106:
URL: https://github.com/apache/beam/pull/12106#discussion_r451179908



##
File path: 
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryServicesImpl.java
##
@@ -793,6 +795,14 @@ public void deleteDataset(String projectId, String 
datasetId)
   String.format(
   "BigQuery insertAll error, retrying: %s",
   
ApiErrorExtractor.INSTANCE.getErrorMessage(e)));
+  GoogleJsonError.ErrorInfo errorInfo = 
getErrorInfo(e);
+  if (errorInfo == null) {
+return insert.execute().getInsertErrors();
+  }
+  if (!ApiErrorExtractor.INSTANCE.rateLimited(e)
+  && 
!errorInfo.getReason().equals("quotaExceeded")) {

Review comment:
   I would move this to a String constant. Also probably add a reference to 
here: https://cloud.google.com/bigquery/docs/error-messages

##
File path: 
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryServicesImpl.java
##
@@ -793,6 +795,14 @@ public void deleteDataset(String projectId, String 
datasetId)
   String.format(
   "BigQuery insertAll error, retrying: %s",
   
ApiErrorExtractor.INSTANCE.getErrorMessage(e)));
+  GoogleJsonError.ErrorInfo errorInfo = 
getErrorInfo(e);
+  if (errorInfo == null) {
+return insert.execute().getInsertErrors();

Review comment:
   Why are we performing an extra retry here ?

##
File path: 
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryServicesImpl.java
##
@@ -893,6 +903,20 @@ public void deleteDataset(String projectId, String 
datasetId)
   ignoreInsertIds);
 }
 
+private GoogleJsonError.ErrorInfo getErrorInfo(IOException e) {
+  GoogleJsonResponseException jsonCause = null;
+  Throwable eCause = e;
+  if (eCause instanceof GoogleJsonResponseException) {

Review comment:
   How do these exceptions extend both GoogleJsonResponseException and 
IOException ?

##
File path: 
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryServicesImpl.java
##
@@ -893,6 +903,20 @@ public void deleteDataset(String projectId, String 
datasetId)
   ignoreInsertIds);
 }
 
+private GoogleJsonError.ErrorInfo getErrorInfo(IOException e) {
+  GoogleJsonResponseException jsonCause = null;
+  Throwable eCause = e;
+  if (eCause instanceof GoogleJsonResponseException) {
+jsonCause = (GoogleJsonResponseException) eCause;
+  } else {
+return null;
+  }
+  GoogleJsonError jsonError = jsonCause.getDetails();

Review comment:
   Please add a unit test for this method.

##
File path: 
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryServicesImpl.java
##
@@ -793,6 +795,14 @@ public void deleteDataset(String projectId, String 
datasetId)
   String.format(
   "BigQuery insertAll error, retrying: %s",
   
ApiErrorExtractor.INSTANCE.getErrorMessage(e)));
+  GoogleJsonError.ErrorInfo errorInfo = 
getErrorInfo(e);
+  if (errorInfo == null) {
+return insert.execute().getInsertErrors();
+  }
+  if (!ApiErrorExtractor.INSTANCE.rateLimited(e)
+  && 
!errorInfo.getReason().equals("quotaExceeded")) {
+return insert.execute().getInsertErrors();

Review comment:
   Ditto regarding the extra retry.





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:
us...@infra.apache.org




[GitHub] [beam] TheNeuralBit commented on a change in pull request #12132: [BEAM-10371] Run dependency check script with Python 3

2020-07-07 Thread GitBox


TheNeuralBit commented on a change in pull request #12132:
URL: https://github.com/apache/beam/pull/12132#discussion_r451186550



##
File path: sdks/python/tox.ini
##
@@ -287,3 +287,12 @@ deps =
 commands =
   yapf --version
   time yapf --diff --parallel --recursive apache_beam
+
+[testenv:py3-dependency-check]
+# TODO: botocore, a part of [interactive], wants docutils<0.16, but Sphinx
+# pulls in the latest docutils. Uncomment this line once botocore does not
+# conflict with Sphinx:
+# extras = docs,test,gcp,aws,interactive,interactive_test
+extras = test,gcp,aws,interactive,interactive_test
+commands =
+  time {toxinidir}/scripts/run_dependency_check.sh

Review comment:
   I think you need to add `passenv = WORKSPACE` here, otherwise the script 
fails because WORKSPACE isn't set.





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:
us...@infra.apache.org




[GitHub] [beam] chamikaramj commented on a change in pull request #12164: [BEAM-10397] add missing environment in windowing strategy for Dataflow

2020-07-07 Thread GitBox


chamikaramj commented on a change in pull request #12164:
URL: https://github.com/apache/beam/pull/12164#discussion_r451184245



##
File path: sdks/python/apache_beam/runners/dataflow/dataflow_runner.py
##
@@ -864,7 +867,7 @@ def run_GroupByKey(self, transform_node, options):
 windowing = transform_node.transform.get_windowing(transform_node.inputs)
 step.add_property(
 PropertyNames.SERIALIZED_FN,
-self.serialize_windowing_strategy(windowing))
+self.serialize_windowing_strategy(windowing, 
self._default_environment))

Review comment:
   Can we always assume default environment here ? What about x-lang 
WindowingStrategies ? Probably this can be tested by running a x-lang GBK test 
with Dataflow with a WindowingStrategy.





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:
us...@infra.apache.org




[GitHub] [beam] TheNeuralBit commented on a change in pull request #12132: [BEAM-10371] Run dependency check script with Python 3

2020-07-07 Thread GitBox


TheNeuralBit commented on a change in pull request #12132:
URL: https://github.com/apache/beam/pull/12132#discussion_r451181865



##
File path: sdks/python/tox.ini
##
@@ -287,3 +287,12 @@ deps =
 commands =
   yapf --version
   time yapf --diff --parallel --recursive apache_beam
+
+[testenv:py3-dependency-check]
+# TODO: botocore, a part of [interactive], wants docutils<0.16, but Sphinx

Review comment:
   Is there a jira for this?





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:
us...@infra.apache.org




[GitHub] [beam] TheNeuralBit merged pull request #12067: [BEAM-10308] Make component ID assignments consistent across PipelineContext instances

2020-07-07 Thread GitBox


TheNeuralBit merged pull request #12067:
URL: https://github.com/apache/beam/pull/12067


   



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:
us...@infra.apache.org




[GitHub] [beam] robinyqiu commented on pull request #11456: [BEAM-7554] Add MillisInstant logical type to replace DATETIME

2020-07-07 Thread GitBox


robinyqiu commented on pull request #11456:
URL: https://github.com/apache/beam/pull/11456#issuecomment-655169701


   cc: @robinyqiu @ZijieSong946 



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:
us...@infra.apache.org




[GitHub] [beam] annaqin418 commented on pull request #12157: [BEAM-7587] Spark portable streaming

2020-07-07 Thread GitBox


annaqin418 commented on pull request #12157:
URL: https://github.com/apache/beam/pull/12157#issuecomment-655169602


   R: @robertwb 
   This PR is still a while from being ready, but just wanted to tag you as 
well in my progress.



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:
us...@infra.apache.org




[GitHub] [beam] robertwb merged pull request #12186: Fix threading issue with default artifact resolver registration.

2020-07-07 Thread GitBox


robertwb merged pull request #12186:
URL: https://github.com/apache/beam/pull/12186


   



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:
us...@infra.apache.org




[GitHub] [beam] amaliujia merged pull request #12169: [BEAM-9953[SQL][ZetaSQL] Support Pure SQL user-defined table-valued function.

2020-07-07 Thread GitBox


amaliujia merged pull request #12169:
URL: https://github.com/apache/beam/pull/12169


   



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:
us...@infra.apache.org




[GitHub] [beam] kennknowles merged pull request #12162: [BEAM-10402] Enable checker framework, disabled everywhere since every module has errors. Eliminate some errors (but no complete modules)

2020-07-07 Thread GitBox


kennknowles merged pull request #12162:
URL: https://github.com/apache/beam/pull/12162


   



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:
us...@infra.apache.org




[GitHub] [beam] pabloem commented on a change in pull request #12107: Interactive Environment Inspector for messaging

2020-07-07 Thread GitBox


pabloem commented on a change in pull request #12107:
URL: https://github.com/apache/beam/pull/12107#discussion_r448112826



##
File path: sdks/python/apache_beam/runners/interactive/utils.py
##
@@ -204,3 +205,23 @@ def run_within_progress_indicator(*args, **kwargs):
   return func(*args, **kwargs)
 
   return run_within_progress_indicator
+
+
+def as_json(func):
+  # type: (Callable[..., Any]) -> Callable[..., Any]

Review comment:
   The callable returns string, right?
   ```suggestion
 # type: (Callable[..., Any]) -> Callable[..., str]
   ```

##
File path: sdks/python/apache_beam/runners/interactive/utils_test.py
##
@@ -195,5 +195,25 @@ def progress_indicated_dummy():
 mocked_javascript.assert_called_once()
 
 
+@unittest.skipIf(
+not ie.current_env().is_interactive_ready,
+'[interactive] dependency is not installed.')
+@unittest.skipIf(
+sys.version_info < (3, 6), 'The tests require at least Python 3.6 to 
work.')
+class MessagingUtilTest(unittest.TestCase):
+  def setUp(self):
+ie.new_env()
+
+  def test_as_json_decorator(self):
+@utils.as_json
+def dummy():
+  return {'a': [1, 2, 3], 'b': 4, 'c': '5', 'd': {'e': 'f'}}
+
+# As of Python 3.6, for the CPython implementation of Python,
+# dictionaries remember the order of items inserted.
+self.assertEqual(
+dummy(), '{"a": [1, 2, 3], "b": 4, "c": "5", "d": {"e": "f"}}')

Review comment:
   nit: you could do something like `json.loads(dummy(), SAMPLE_DATA)` 
instead of depending on the ordering.
   ```suggestion
   json.loads(dummy()), {"a": [1, 2, 3], "b": 4, "c": "5", "d": {"e": 
"f"}})
   ```





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:
us...@infra.apache.org




[GitHub] [beam] pabloem commented on pull request #12107: Interactive Environment Inspector for messaging

2020-07-07 Thread GitBox


pabloem commented on pull request #12107:
URL: https://github.com/apache/beam/pull/12107#issuecomment-655161085


   left just two comments. Please respond (address if you think it's worth 
addressing) - and we can merge



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:
us...@infra.apache.org




[GitHub] [beam] y1chi commented on pull request #11339: [BEAM-9468] Fhir io

2020-07-07 Thread GitBox


y1chi commented on pull request #11339:
URL: https://github.com/apache/beam/pull/11339#issuecomment-655160801


   @jaketf the integration tests seem to be flaky and breaks postcommit 
sometimes, I opened https://issues.apache.org/jira/browse/BEAM-10419 do you 
mind take a look?



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:
us...@infra.apache.org




[GitHub] [beam] chamikaramj commented on pull request #12188: [BEAM-10411] Adds an example that use Python cross-language Kafka transforms.

2020-07-07 Thread GitBox


chamikaramj commented on pull request #12188:
URL: https://github.com/apache/beam/pull/12188#issuecomment-655159675


   cc: @davidwrede 



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:
us...@infra.apache.org




[GitHub] [beam] chamikaramj commented on pull request #12067: [BEAM-10308] Make component ID assignments consistent across PipelineContext instances

2020-07-07 Thread GitBox


chamikaramj commented on pull request #12067:
URL: https://github.com/apache/beam/pull/12067#issuecomment-655155939


   Took a look and confirmed that x-lang KafkaIO works with this change. LGTM 
from me.



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:
us...@infra.apache.org




[GitHub] [beam] robertwb commented on pull request #12186: Fix threading issue with default artifact resolver registration.

2020-07-07 Thread GitBox


robertwb commented on pull request #12186:
URL: https://github.com/apache/beam/pull/12186#issuecomment-655145852


   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:
us...@infra.apache.org




[GitHub] [beam] jaketf commented on pull request #11959: refactor HCLS IO ITs to support stores in other projects

2020-07-07 Thread GitBox


jaketf commented on pull request #11959:
URL: https://github.com/apache/beam/pull/11959#issuecomment-655145391


   I have some level of access but I need additional permissions to modify IAM



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:
us...@infra.apache.org




[GitHub] [beam] piotr-szuberski commented on pull request #12145: [BEAM-10136] [BEAM-10135] Add JdbcIO for cross-language with python wrapper

2020-07-07 Thread GitBox


piotr-szuberski commented on pull request #12145:
URL: https://github.com/apache/beam/pull/12145#issuecomment-655122687







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:
us...@infra.apache.org




[GitHub] [beam] piotr-szuberski removed a comment on pull request #12145: [BEAM-10136] [BEAM-10135] Add JdbcIO for cross-language with python wrapper

2020-07-07 Thread GitBox


piotr-szuberski removed a comment on pull request #12145:
URL: https://github.com/apache/beam/pull/12145#issuecomment-655122687


   Run XVR_Spark PostCommitRun XVR_Spark 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:
us...@infra.apache.org




[GitHub] [beam] lukecwik commented on a change in pull request #12184: [BEAM-10392] Make sure that messages that are unroutable are returned to the sender and redelivered.

2020-07-07 Thread GitBox


lukecwik commented on a change in pull request #12184:
URL: https://github.com/apache/beam/pull/12184#discussion_r451123739



##
File path: 
sdks/java/io/rabbitmq/src/test/java/org/apache/beam/sdk/io/rabbitmq/RabbitMqIOTest.java
##
@@ -196,59 +200,74 @@ private void doExchangeTest(ExchangeTestPlan testPlan, 
boolean simulateIncompati
 exchangeType = "fanout";
   }
 }
+final String finalExchangeType = exchangeType;
+final CountDownLatch waitForExchangeToBeDeclared = new CountDownLatch(1);
+final BlockingQueue recordsToPublish = new LinkedBlockingQueue<>();
+
recordsToPublish.addAll(RabbitMqTestUtils.generateRecords(testPlan.getNumRecordsToPublish()));
+Thread publisher =
+new Thread(
+() -> {
+  Connection connection = null;
+  Channel channel = null;
+  try {
+ConnectionFactory connectionFactory = new ConnectionFactory();
+connectionFactory.setAutomaticRecoveryEnabled(false);
+connectionFactory.setUri(uri);
+connection = connectionFactory.newConnection();
+channel = connection.createChannel();
+channel.exchangeDeclare(exchange, finalExchangeType);
+// We are relying on the pipeline to declare the queue and 
messages that are
+// published without a queue being declared are "unroutable". 
Since there is a race
+// between when the pipeline declares and when we can start 
publishing, we add a
+// handler to republish messages that are returned to us.
+channel.addReturnListener(
+(replyCode, replyText, exchange1, routingKey, properties, 
body) -> {
+  try {
+recordsToPublish.put(body);
+  } catch (Exception e) {
+throw new RuntimeException(e);
+  }
+});
+waitForExchangeToBeDeclared.countDown();
+while (true) {
+  byte[] record = recordsToPublish.take();
+  if (record == terminalRecord) {
+return;
+  }
+  channel.basicPublish(
+  exchange,
+  testPlan.publishRoutingKeyGen().get(),
+  true, // ensure that messages are returned to sender
+  testPlan.getPublishProperties(),
+  record);
+}
 
-ConnectionFactory connectionFactory = new ConnectionFactory();
-connectionFactory.setAutomaticRecoveryEnabled(false);
-connectionFactory.setUri(uri);
-Connection connection = null;
-Channel channel = null;
-
-try {
-  connection = connectionFactory.newConnection();
-  channel = connection.createChannel();
-  channel.exchangeDeclare(exchange, exchangeType);
-  final Channel finalChannel = channel;
-  Thread publisher =
-  new Thread(
-  () -> {
-try {
-  Thread.sleep(5000);
-} catch (Exception e) {
-  LOG.error(e.getMessage(), e);
+  } catch (Exception e) {
+throw new RuntimeException(e);
+  } finally {
+if (channel != null) {
+  // channel may have already been closed automatically due to 
protocol failure
+  try {
+channel.close();
+  } catch (Exception e) {
+/* ignored */
+  }
 }
-for (int i = 0; i < testPlan.getNumRecordsToPublish(); i++) {
+if (connection != null) {
+  // connection may have already been closed automatically due 
to protocol failure
   try {
-finalChannel.basicPublish(
-exchange,
-testPlan.publishRoutingKeyGen().get(),
-testPlan.getPublishProperties(),
-RabbitMqTestUtils.generateRecord(i));
+connection.close();
   } catch (Exception e) {
-LOG.error(e.getMessage(), e);
+/* ignored */
   }
 }
-  });
-  publisher.start();
-  p.run();
-  publisher.join();
-} finally {
-  if (channel != null) {
-// channel may have already been closed automatically due to protocol 
failure
-try {
-  channel.close();
-} catch (Exception e) {
-  /* ignored */
-}
-  }
-  if (connection != null) {
-// connection may have already been closed automatically due to 
protocol failure
-try {
-  connection.close();
-} catch (Exception e) {
-  /* ignored */
-}
-  }

[GitHub] [beam] lukecwik merged pull request #12089: [BEAM-10283] Add new overloads of withKeyRanges and withRowFilter met…

2020-07-07 Thread GitBox


lukecwik merged pull request #12089:
URL: https://github.com/apache/beam/pull/12089


   



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:
us...@infra.apache.org




[GitHub] [beam] lukecwik commented on a change in pull request #12089: [BEAM-10283] Add new overloads of withKeyRanges and withRowFilter met…

2020-07-07 Thread GitBox


lukecwik commented on a change in pull request #12089:
URL: https://github.com/apache/beam/pull/12089#discussion_r451120512



##
File path: 
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigtable/BigtableIO.java
##
@@ -410,13 +417,22 @@ public Read withKeyRange(ByteKeyRange keyRange) {
  *
  * Does not modify this object.
  */
-public Read withKeyRanges(List keyRanges) {
+public Read withKeyRanges(ValueProvider> keyRanges) {
   checkArgument(keyRanges != null, "keyRanges can not be null");
-  checkArgument(!keyRanges.isEmpty(), "keyRanges can not be empty");
-  for (ByteKeyRange range : keyRanges) {
-checkArgument(range != null, "keyRanges cannot hold null range");
-  }
-  return toBuilder().setKeyRanges(keyRanges).build();
+  BigtableReadOptions bigtableReadOptions = getBigtableReadOptions();
+  return toBuilder()
+  
.setBigtableReadOptions(bigtableReadOptions.toBuilder().setKeyRanges(keyRanges).build())
+  .build();
+}
+
+/**
+ * Returns a new {@link BigtableIO.Read} that will read only rows in the 
specified ranges.
+ * Ranges must not overlap.

Review comment:
   I'm not aware of a JIRA, you'll need to look. If you don't find one, 
file one.





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:
us...@infra.apache.org




[GitHub] [beam] pabloem merged pull request #12190: Fix typos in programming guide

2020-07-07 Thread GitBox


pabloem merged pull request #12190:
URL: https://github.com/apache/beam/pull/12190


   



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:
us...@infra.apache.org




[GitHub] [beam] pabloem commented on pull request #12190: Fix typos in programming guide

2020-07-07 Thread GitBox


pabloem commented on pull request #12190:
URL: https://github.com/apache/beam/pull/12190#issuecomment-655092319


   Thanks! LGTM. I'll merge after website precommit passes



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:
us...@infra.apache.org




[GitHub] [beam] aaltay commented on pull request #12128: [BEAM-10417] - Move Shared object from tfx_bsl

2020-07-07 Thread GitBox


aaltay commented on pull request #12128:
URL: https://github.com/apache/beam/pull/12128#issuecomment-655091453


   > Optional comments:
   > 
   > * is there an associated BEAM jira issue that can be used for the commit?
   > * consider adding this to the CHANGES.md.
   
   Done. Thank you!



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:
us...@infra.apache.org




[GitHub] [beam] ceh commented on pull request #12190: Fix typos in programming guide

2020-07-07 Thread GitBox


ceh commented on pull request #12190:
URL: https://github.com/apache/beam/pull/12190#issuecomment-655090603


   R: @aaltay, @pabloem 



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:
us...@infra.apache.org




[GitHub] [beam] rohdesamuel commented on pull request #12182: [BEAM-4605] Remove portable runner GBK hack.

2020-07-07 Thread GitBox


rohdesamuel commented on pull request #12182:
URL: https://github.com/apache/beam/pull/12182#issuecomment-655090706


   LGTM, thanks for the cleanup!



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:
us...@infra.apache.org




[GitHub] [beam] ceh opened a new pull request #12190: Fix typos in programming guide

2020-07-07 Thread GitBox


ceh opened a new pull request #12190:
URL: https://github.com/apache/beam/pull/12190


   Fix a couple of minor typos in the programming guide.
   
   
   
   Thank you for your contribution! Follow this checklist to help us 
incorporate your contribution quickly and easily:
   
- [ ] [**Choose 
reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and 
mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in 
ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA 
issue, if applicable. This will automatically link the pull request to the 
issue.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual 
Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   See the [Contributor Guide](https://beam.apache.org/contribute) for more 
tips on [how to make review process 
smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
   
   Post-Commit Tests Status (on master branch)
   

   
   Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
   --- | --- | --- | --- | --- | --- | ---
   Go | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/)
 | --- | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/)
 | --- | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
 | ---
   Java | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/)
 | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
 | [![Build 
Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
   Python | [![Build 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)[![Build
 
Status](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)[![Build
 

[GitHub] [beam] TheNeuralBit merged pull request #12090: [BEAM-10336,BEAM-10337] Add SchemaIO abstraction and implement for PubSub

2020-07-07 Thread GitBox


TheNeuralBit merged pull request #12090:
URL: https://github.com/apache/beam/pull/12090


   



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:
us...@infra.apache.org




[GitHub] [beam] aaltay commented on pull request #12128: Move Shared object from tfx_bsl

2020-07-07 Thread GitBox


aaltay commented on pull request #12128:
URL: https://github.com/apache/beam/pull/12128#issuecomment-655085033


   Run Python 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:
us...@infra.apache.org




[GitHub] [beam] tvalentyn merged pull request #12180: [BEAM-9754] Adds Python 3.8 ValidatesRunner tests for Dataflow.

2020-07-07 Thread GitBox


tvalentyn merged pull request #12180:
URL: https://github.com/apache/beam/pull/12180


   



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:
us...@infra.apache.org




[GitHub] [beam] pabloem commented on pull request #12183: Fix a class cast bug: TupleTagList is not a collection.

2020-07-07 Thread GitBox


pabloem commented on pull request #12183:
URL: https://github.com/apache/beam/pull/12183#issuecomment-655066457


   Thanks @lastomato !



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:
us...@infra.apache.org




[GitHub] [beam] pabloem merged pull request #12183: Fix a class cast bug: TupleTagList is not a collection.

2020-07-07 Thread GitBox


pabloem merged pull request #12183:
URL: https://github.com/apache/beam/pull/12183


   



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:
us...@infra.apache.org




[GitHub] [beam] pabloem commented on a change in pull request #12143: [BEAM-10291] Adding full thread dump upon lull detection for Dataflow…

2020-07-07 Thread GitBox


pabloem commented on a change in pull request #12143:
URL: https://github.com/apache/beam/pull/12143#discussion_r451078998



##
File path: 
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/DataflowOperationContext.java
##
@@ -194,6 +200,18 @@ public DataflowExecutionState(
   this.metricsContainer = metricsContainer;
 }
 
+public DataflowExecutionState(
+NameContext nameContext,
+String stateName,
+@Nullable String requestingStepName,
+@Nullable Integer inputIndex,
+@Nullable MetricsContainer metricsContainer,
+ProfileScope profileScope,
+Clock clock) {
+  this(nameContext, stateName, requestingStepName, inputIndex, 
metricsContainer, profileScope);
+  this.clock = clock;
+}

Review comment:
   Instead, add Clock to the constructor above (lines 188-200), make the 
clock member final, and pass Clock.SYSTEM in the constructor without it?

##
File path: 
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/DataflowOperationContext.java
##
@@ -264,7 +276,49 @@ public void reportLull(Thread trackedThread, long millis) {
   logRecord.setLoggerName(DataflowOperationContext.LOG.getName());
 
   // Publish directly in the context of this specific ExecutionState.
-  DataflowWorkerLoggingInitializer.getLoggingHandler().publish(this, 
logRecord);
+  DataflowWorkerLoggingHandler dataflowLoggingHandler =
+  DataflowWorkerLoggingInitializer.getLoggingHandler();
+  dataflowLoggingHandler.publish(this, logRecord);
+
+  if (shouldLogFullThreadDump()) {
+Map threadSet = 
Thread.getAllStackTraces();
+for (Map.Entry entry : 
threadSet.entrySet()) {
+  Thread thread = entry.getKey();
+  StackTraceElement[] stackTrace = entry.getValue();
+  StringBuilder message = new StringBuilder();
+  message.append(thread.toString()).append(":\n");
+  message.append(getStackTraceForLullMessage(stackTrace));
+  logRecord = new LogRecord(Level.INFO, message.toString());
+  logRecord.setLoggerName(DataflowOperationContext.LOG.getName());
+  dataflowLoggingHandler.publish(this, logRecord);
+}
+  }
+}
+
+// A full thread dump is performed at most once every 20 minutes.
+private static final long LOG_LULL_FULL_THREAD_DUMP_MS = 20 * 60 * 1000;
+
+// Last time when a full thread dump was performed.
+private long lastFullThreadDumpMillis = 0;
+
+private boolean shouldLogFullThreadDump() {

Review comment:
   This means that a full thread dump will happen the very first time a 
lull is logged. Lulls are not that unusual, right? We want full thread dumps 
for pipelines that are truly stuck, not for pipelines that are experiencing a 
slow stage.
   
   Perhaps we should use the `millis` argument from `reportLull`, and only 
report when millis passes 20 minutes?





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:
us...@infra.apache.org




[GitHub] [beam] pabloem commented on pull request #12143: [BEAM-10291] Adding full thread dump upon lull detection for Dataflow…

2020-07-07 Thread GitBox


pabloem commented on pull request #12143:
URL: https://github.com/apache/beam/pull/12143#issuecomment-655059029







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:
us...@infra.apache.org




[GitHub] [beam] robertwb commented on pull request #12186: Fix threading issue with default artifact resolver registration.

2020-07-07 Thread GitBox


robertwb commented on pull request #12186:
URL: https://github.com/apache/beam/pull/12186#issuecomment-655056238


   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:
us...@infra.apache.org




[GitHub] [beam] apilloud merged pull request #12159: [BEAM-10093] Run ZetaSQL Nexmark in postcommit

2020-07-07 Thread GitBox


apilloud merged pull request #12159:
URL: https://github.com/apache/beam/pull/12159


   



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:
us...@infra.apache.org




[GitHub] [beam] aaltay commented on pull request #12128: Move Shared object from tfx_bsl

2020-07-07 Thread GitBox


aaltay commented on pull request #12128:
URL: https://github.com/apache/beam/pull/12128#issuecomment-655056227


   Run Python 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:
us...@infra.apache.org




[GitHub] [beam] TheNeuralBit commented on pull request #12090: [BEAM-10336,BEAM-10337] Add SchemaIO abstraction and implement for PubSub

2020-07-07 Thread GitBox


TheNeuralBit commented on pull request #12090:
URL: https://github.com/apache/beam/pull/12090#issuecomment-655052483


   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:
us...@infra.apache.org




[GitHub] [beam] davidyan74 commented on pull request #12143: [BEAM-10291] Adding full thread dump upon lull detection for Dataflow…

2020-07-07 Thread GitBox


davidyan74 commented on pull request #12143:
URL: https://github.com/apache/beam/pull/12143#issuecomment-655047439


   Looks like another test failed and looks unrelated again. @pabloem Do you 
think this PR could make the tests more flaky? 



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:
us...@infra.apache.org




[GitHub] [beam] aaltay commented on a change in pull request #12166: [BEAM-10404] Cancel queued/running GitHub Action builds on second push to PR

2020-07-07 Thread GitBox


aaltay commented on a change in pull request #12166:
URL: https://github.com/apache/beam/pull/12166#discussion_r451063093



##
File path: .github/workflows/cancel.yml
##
@@ -16,7 +16,7 @@
 # under the License.
 
 name: Cancel
-on: [push]
+on: [push, pull_request]

Review comment:
   > e.g. new merge commit on master appear. So it is possible to run rests 
without PR.
   I do not understand how/when this would happen. Because we do not allow 
direct pushes to master. And if it is already in master a PR would not be 
helpful. Maybe this example applies to release branches?
   
   Overall, I am fine with the change. It is ok to cancel tests when a new PR 
is created. 





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:
us...@infra.apache.org




[GitHub] [beam] chamikaramj commented on pull request #12060: [BEAM-10218] Add FnApiRunner to cross-language validate runner test

2020-07-07 Thread GitBox


chamikaramj commented on pull request #12060:
URL: https://github.com/apache/beam/pull/12060#issuecomment-655044889


   LGTM. Thanks. Could yo resolve the conflict ?
   
   Also, can we trigger tests from here to see if this works or are there more 
missing pieces ?



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:
us...@infra.apache.org




[GitHub] [beam] tvalentyn commented on pull request #12179: [BEAM-5414] - Update grpcio-tools to its latest version.

2020-07-07 Thread GitBox


tvalentyn commented on pull request #12179:
URL: https://github.com/apache/beam/pull/12179#issuecomment-655039855


   cc: @robertwb @angoenka re: 
https://github.com/apache/beam/pull/12179#issuecomment-654548592.
   Do we need an AI to suppress such errors?



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:
us...@infra.apache.org




[GitHub] [beam] tvalentyn commented on pull request #12179: [BEAM-5414] - Update grpcio-tools to its latest version.

2020-07-07 Thread GitBox


tvalentyn commented on pull request #12179:
URL: https://github.com/apache/beam/pull/12179#issuecomment-655038045


   If this happened during interpreter shutdown perhaps all bets are off? 



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:
us...@infra.apache.org




[GitHub] [beam] chamikaramj commented on pull request #11919: [BEAM-10114] Copy Pub/Sub Lite IO from Pub/Sub Lite github to beam.

2020-07-07 Thread GitBox


chamikaramj commented on pull request #11919:
URL: https://github.com/apache/beam/pull/11919#issuecomment-655035073


   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:
us...@infra.apache.org




[GitHub] [beam] pabloem commented on pull request #12183: Fix a class cast bug: TupleTagList is not a collection.

2020-07-07 Thread GitBox


pabloem commented on pull request #12183:
URL: https://github.com/apache/beam/pull/12183#issuecomment-655011572


   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:
us...@infra.apache.org




[GitHub] [beam] pabloem commented on pull request #12143: [BEAM-10291] Adding full thread dump upon lull detection for Dataflow…

2020-07-07 Thread GitBox


pabloem commented on pull request #12143:
URL: https://github.com/apache/beam/pull/12143#issuecomment-655011390


   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:
us...@infra.apache.org




[GitHub] [beam] pabloem commented on pull request #12183: Fix a class cast bug: TupleTagList is not a collection.

2020-07-07 Thread GitBox


pabloem commented on pull request #12183:
URL: https://github.com/apache/beam/pull/12183#issuecomment-655011463


   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:
us...@infra.apache.org




[GitHub] [beam] pabloem commented on pull request #12086: [BEAM-10322] allow only single assignment to producing stages by pcol…

2020-07-07 Thread GitBox


pabloem commented on pull request #12086:
URL: https://github.com/apache/beam/pull/12086#issuecomment-655006468


   My bad. Heejong and I discussed it via chat last week. I forgot to add the
   LGTM when I merged.
   
   On Tue, Jul 7, 2020, 9:36 AM Ahmet Altay  wrote:
   
   > Folks just checking. I do not see an LGTM or an approval on this PR. Is
   > everyone OK with it being merged?
   >
   > —
   > You are receiving this because you modified the open/close state.
   > Reply to this email directly, view it on GitHub
   > , or
   > unsubscribe
   > 

   > .
   >
   



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:
us...@infra.apache.org




[GitHub] [beam] robertwb commented on pull request #12186: Fix threading issue with default artifact resolver registration.

2020-07-07 Thread GitBox


robertwb commented on pull request #12186:
URL: https://github.com/apache/beam/pull/12186#issuecomment-655002473


   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:
us...@infra.apache.org




[GitHub] [beam] robertwb commented on pull request #12186: Fix threading issue with default artifact resolver registration.

2020-07-07 Thread GitBox


robertwb commented on pull request #12186:
URL: https://github.com/apache/beam/pull/12186#issuecomment-655002395


   This did come up in a test flake. 



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:
us...@infra.apache.org




[GitHub] [beam] davidyan74 commented on pull request #12143: [BEAM-10291] Adding full thread dump upon lull detection for Dataflow…

2020-07-07 Thread GitBox


davidyan74 commented on pull request #12143:
URL: https://github.com/apache/beam/pull/12143#issuecomment-654993643


   Looks like the test didn't get triggered.



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:
us...@infra.apache.org




[GitHub] [beam] piotr-szuberski commented on pull request #12145: [BEAM-10136] [BEAM-10135] Add JdbcIO for cross-language with python wrapper

2020-07-07 Thread GitBox


piotr-szuberski commented on pull request #12145:
URL: https://github.com/apache/beam/pull/12145#issuecomment-654992334


   Run XVR_Spark 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:
us...@infra.apache.org




[GitHub] [beam] aaltay merged pull request #12179: [BEAM-5414] - Update grpcio-tools to its latest version.

2020-07-07 Thread GitBox


aaltay merged pull request #12179:
URL: https://github.com/apache/beam/pull/12179


   



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:
us...@infra.apache.org




[GitHub] [beam] aaltay commented on pull request #12086: [BEAM-10322] allow only single assignment to producing stages by pcol…

2020-07-07 Thread GitBox


aaltay commented on pull request #12086:
URL: https://github.com/apache/beam/pull/12086#issuecomment-654982821


   Folks just checking. I do not see an LGTM or an approval on this PR. Is 
everyone OK with it being 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.

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




[GitHub] [beam] tvalentyn commented on pull request #12180: Adds Python 3.8 ValidatesRunner tests for Dataflow.

2020-07-07 Thread GitBox


tvalentyn commented on pull request #12180:
URL: https://github.com/apache/beam/pull/12180#issuecomment-654974496


   R: @aaltay @kamilwu @lazylynx 



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:
us...@infra.apache.org




[GitHub] [beam] tvalentyn commented on a change in pull request #12150: [BEAM-10398] Use GitHub Actions in wheels release process for Python

2020-07-07 Thread GitBox


tvalentyn commented on a change in pull request #12150:
URL: https://github.com/apache/beam/pull/12150#discussion_r450986594



##
File path: release/src/main/python-release/python_release_automation.sh
##
@@ -19,7 +19,7 @@
 source 
release/src/main/python-release/run_release_candidate_python_quickstart.sh
 source 
release/src/main/python-release/run_release_candidate_python_mobile_gaming.sh
 
-for version in 2.7 3.5 3.6 3.7
+for version in 2.7 3.5 3.6 3.7 3.8

Review comment:
   Please keep Py3.8, we should assume Py3.8 to be a supported version and 
we will release artifacts for Py38 with next release.





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:
us...@infra.apache.org




[GitHub] [beam] tysonjh commented on pull request #12128: Move Shared object from tfx_bsl

2020-07-07 Thread GitBox


tysonjh commented on pull request #12128:
URL: https://github.com/apache/beam/pull/12128#issuecomment-654963681


   Optional comments:
 * is there an associated BEAM jira issue that can be used for the commit?
 * consider adding this to the CHANGES.md.



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:
us...@infra.apache.org




[GitHub] [beam] tysonjh commented on pull request #12128: Move Shared object from tfx_bsl

2020-07-07 Thread GitBox


tysonjh commented on pull request #12128:
URL: https://github.com/apache/beam/pull/12128#issuecomment-654962545


   LGTM - thank you!



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:
us...@infra.apache.org




[GitHub] [beam] lastomato commented on pull request #12183: Fix a class cast bug: TupleTagList is not a collection.

2020-07-07 Thread GitBox


lastomato commented on pull request #12183:
URL: https://github.com/apache/beam/pull/12183#issuecomment-654962111


   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:
us...@infra.apache.org




[GitHub] [beam] mxm merged pull request #12133: [BEAM-10385] Integrate SQL expansion into Flink job server

2020-07-07 Thread GitBox


mxm merged pull request #12133:
URL: https://github.com/apache/beam/pull/12133


   



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:
us...@infra.apache.org




[GitHub] [beam] piotr-szuberski edited a comment on pull request #12145: [BEAM-10136] [BEAM-10135] Add JdbcIO for cross-language with python wrapper

2020-07-07 Thread GitBox


piotr-szuberski edited a comment on pull request #12145:
URL: https://github.com/apache/beam/pull/12145#issuecomment-654901752


   Yeah, I noticed that it times out.
   
   Thanks that you tried to run the tests on your setup. Locally on my computer 
`:runners:flink:1.10:job-server:validatesCrossLanguageRunner` passes and stops 
without a hassle, I don't get what is the reason the `XVR_Flink PostCommit` 
fails.
   
   Especially that `Python PreCommit` also runs those tests and they pass.
   
   Tomorrow I'll try to run them on Ubuntu, maybe OSX is the problem. But I 
doubt it.



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:
us...@infra.apache.org




[GitHub] [beam] piotr-szuberski edited a comment on pull request #12145: [BEAM-10136] [BEAM-10135] Add JdbcIO for cross-language with python wrapper

2020-07-07 Thread GitBox


piotr-szuberski edited a comment on pull request #12145:
URL: https://github.com/apache/beam/pull/12145#issuecomment-654901752


   Yeah, I noticed that it times out.
   
   Thanks that you tried to run the tests on your setup. Locally on my computer 
`:runners:flink:1.10:job-server:validatesCrossLanguageRunner` passes and stops 
without a hassle, I don't get what is the reason the `XVR_Flink PostCommit` 
fails.
   
   Especially that `Python PreCommit` also runs those tests and they pass.
   
   Tomorrow I'll try to run them on Ubuntu, maybe OSX is the problem.. but I 
doubt it.



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:
us...@infra.apache.org




  1   2   >